Fixes T7480, File names should be editable and the event should show up in feed.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7480: Allow editing the name of a file
- Commits
- Restricted Diffusion Commit
rP55ff197f2a89: File names should be editable.
Upload a file, view file details, edit file, change file name by adding a space and a word to the name, save changes, file name should retain space and not normalize the name, file details should show the edit event, install feed should correctly show an event for the action.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Couple of minor inlines.
We should prevent empty names, per discussion.
For questionable names which don't survive normalization, let's do this:
- Overall, these steps will move us from unevenly normalizing inputs to never normalizing inputs and always normalizing outputs.
- Remove the call to normalizeFileName() in PhabricatorFile->buildFromFileDataOrHash().
- Remove the call to normalizeFileName() in PhabricatorFile->readPropertiesFromParameters().
- Add a call to normalizeFileName() in PhabricatorFile->getCDNURI().
This will let files have silly names within Phabricator, but make sure we can generate working URIs even if the name is a creative one.
| src/applications/files/storage/PhabricatorFileTransaction.php | ||
|---|---|---|
| 29 | It's a little more conventional to just put the strings in this transaction description, like: return pht( '%s updated the name for this file from "%s" to "%s".', $this->renderHandleLink($author_phid), $old, $new); Then remove hasChangeDetails() and renderChangeDetails(). This will just show the change in the transaction instead of requiring a click. The click workflow is used elsewhere for fields which may contain a lot of text, like a description or document, so we can't just embed the entire change in the transaction. | |
| 48 | Just dump the names in here too. | |
| src/applications/files/storage/PhabricatorFile.php | ||
|---|---|---|
| 696 | Tested this and it's not actually normalizing the name. Definitely missing something. | |
| src/applications/files/storage/PhabricatorFile.php | ||
|---|---|---|
| 695 | $name isn't declared here yet, so PHP fatals:
| |