Page MenuHomePhorge

Add and use new RemarkupMetadata class
Needs RevisionPublic

Authored by Dylsss on Sep 18 2022, 16:59.
Referenced Files
Unknown Object (File)
Sat, Feb 4, 07:29
Unknown Object (File)
Sat, Jan 28, 18:13
Unknown Object (File)
Fri, Jan 27, 21:53
Unknown Object (File)
Fri, Jan 27, 21:53
Unknown Object (File)
Fri, Jan 27, 21:52
Unknown Object (File)
Fri, Jan 27, 21:52
Unknown Object (File)
Wed, Jan 25, 05:49
Unknown Object (File)
Sun, Jan 22, 01:05

Details

Summary

Add a new RemarkupMetadata class and use with upload button as well as drag and drop pathways.

Test Plan

Drag and drop file, upload file with button. Check that both files are attached with "attachedFilePHIDs" values.

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningwebroot/rsrc/js/core/RemarkupMetadata.js:1JAVELIN5`javelinsymbols` Not In Path
Unit
Tests Passed
Build Status
Buildable 87
Build 87: arc lint + arc unit

Event Timeline

Dylsss requested review of this revision.Sep 18 2022, 16:59

Differentiate between metadata of different text areas using node ID. e.g., new comment text areas and text areas in the edit comment dialog.

Bear with me here, because Javeline (like all js frameworks) is confusing to me.

I'd expect some PHP code to actually attach the files from attachedFilePHIDs to the relevant object, but I don't see it in this diff. Is that already happening somehow?

webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js
260–264

is metadata gets automatically inserted into something? I don't see it being manually saved.

In D25052#1750, @avivey wrote:

I'd expect some PHP code to actually attach the files from attachedFilePHIDs to the relevant object, but I don't see it in this diff. Is that already happening somehow?

It's done here: https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php;5aa159a83013fca3a72f5c6d2c493c117fbbf83b$2296

webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js
260–264

It gets written to the HTML element in the setMetadata method.

This revision is now accepted and ready to land.Oct 28 2022, 10:19
Matthew requested changes to this revision.Dec 8 2022, 22:37

Please rebase and resubmit, there is a merge conflict.

matthew@Tower phorge % arc patch D25052
Created and checked out branch arcpatch-D25052.
Checking patch webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js...
Checking patch webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js...
Checking patch webroot/rsrc/js/core/RemarkupMetadata.js...
Checking patch src/view/form/control/PhabricatorRemarkupControl.php...
Checking patch resources/celerity/map.php...
Applied patch webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js cleanly.
Applied patch webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js cleanly.
Applied patch webroot/rsrc/js/core/RemarkupMetadata.js cleanly.
Applied patch src/view/form/control/PhabricatorRemarkupControl.php cleanly.
Applied patch resources/celerity/map.php cleanly.
 COMMITTED  Successfully committed patch.
matthew@Tower phorge % arc land
 STRATEGY  Merging with "squash" strategy, the default strategy.
 SOURCE  Landing the current branch, "arcpatch-D25052".
 ONTO REMOTE  Landing onto remote "origin", the default remote under Git.
 ONTO TARGET  Landing onto target "master", the default target under Git.
 INTO REMOTE  Will merge into remote "origin" by default, because this is the remote the change is landing onto.
 INTO TARGET  Will merge into target "master" by default, because this is the "onto" target.
 FETCH  Fetching "master" from remote "origin"...

  $   git fetch --no-tags --quiet -- origin master


 INTO COMMIT  Preparing merge into "master" from remote "origin", at commit "dc558b5538cd".
 LANDING  These changes will land:

  *   D25052 Add and use new RemarkupMetadata class
          27935e60780f  Add and use new RemarkupMetadata class

 >>>  Land these changes? [y/N/?] y

 <!> NOT REVISION AUTHOR 
You are landing revisions which you ("@Matthew") are not the author of:

  *   D25052 Add and use new RemarkupMetadata class
        Author: @Dylsss

  ?   Use "Commandeer" in the web interface to become the author of a
      revision.

 >>>  Land revisions you are not the author of? [y/N/?] y
 <!> BUILD FAILURES 
1 revision(s) have build failures:

  *   D25052 Add and use new RemarkupMetadata class
  *     B87 Buildable "B87"
 ://      https://we.phorge.it/B87
  *       Build 87 arc lint + arc unit


 >>>  Land 1 revision(s) anyway, despite failed builds? [y/N/?] y
 MERGING  27935e60780f Add and use new RemarkupMetadata class
 MERGE  Attempting to rebase changes.
 MERGE  Attempting to reduce and rebase changes.

 <!> MERGE CONFLICT 
Local commit "27935e60780f" ("arcpatch-D25052") does not merge cleanly into
"dc558b5538cd". Rebase or merge local changes so they can merge cleanly.

 LOAD STATE  Restoring local state (to ref "arcpatch-D25052" at commit "27935e60780f").
 USAGE EXCEPTION  Encountered a merge conflict.
This revision now requires changes to proceed.Dec 8 2022, 22:37