When you drag and drop a file in a Conpherence chat,
the file was only visible by the author.
Now the file is also attached to that chat, making it visible.
This is a follow-up from:
https://we.phorge.it/D25705?id=2178
Refs T15106
Details
Details
- Reviewers
valerio.bozzolan - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15106: "Upload file" in remarkup text fields should attach by default
- Commits
- rP136091ecd097: Fix missing file attachment in Conpherence
- open a Conpherence chat or create a new one
- drag and drop file in it and send the message
- verify file is attached to the chat
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- arcpatch-D25709
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1425 Build 1425: arc lint + arc unit
Event Timeline
Comment Actions
To avoid that $xactions[0] we should probably find the desired transaction.
Note: At the moment we lack a similar commodity method to find transactions, but that would be probably appreciated:
$xaction_comment = PhabricatorTransactions::findOneByType( $xactions, PhabricatorTransactions::TYPE_COMMENT);
Here what I'm talking about adopting findOneByType():
https://we.phorge.it/D25710?vs=2188&id=2189#toc
This new method "find transaction by type" opens some creative space to apply some minor refactors here and there:
and other things related to grep -F -C10 -R 'foreach ($xactions as $x' . | grep "if.*Type.*==" -C 4
| src/applications/conpherence/controller/ConpherenceUpdateController.php | ||
|---|---|---|
| 100 | The xactions[0] is a bit hardcoded. In the future the generateTransactionsFromText() may return different elements, and the transaction related to comments may be in a different position. Maybe better to find the one with type PhabricatorTransactions::TYPE_COMMENT. | |
Comment Actions
P.S. Why does Phriction have the field text_metadata? And why does Conpherence have content_metadata? 🤔
Comment Actions
For Conpherence, the content is stored in a property named text and the file PHIDs are stored in the corresponding text_metadata.
Maniphest tasks have description and description_metadata.
it's the other way around.
When you save a Phriction document, the content is stored in the content property.
There is also a content_metadata property created with this action, which contains the attachedFilePHIDs.
This is the network logging from Firefox for a Phriction wiki document update:
For Conpherence, the content is stored in a property named text and the file PHIDs are stored in the corresponding text_metadata.
Maniphest tasks have description and description_metadata.
Comment Actions
OK.
@avivey what do you think about a new silly commodity method PhabricatorTransactions::findOneByType(array, string) ?
Maybe you have a strong opinion
Comment Actions
Please wait at least for another +1, or, a week, before landing this. Since others may have different opinions on PhabricatorTransactions::findOneByType().
Thanks, tested, finally I share a file and people see it also in Conpherence 🌈 awesome
Comment Actions
I've seen worse 🤷🏻♂️
| src/applications/conpherence/controller/ConpherenceUpdateController.php | ||
|---|---|---|
| 90 | linebreak before the first argument too | |
Comment Actions
Double-slam-accept! Thanks and happy landing
| src/applications/conpherence/controller/ConpherenceUpdateController.php | ||
|---|---|---|
| 88 | ||