Fixes T11476.
Details
- Installed/uninstalled the Conpherence application
- Observed correct timeline stories
- Observed correct config in database
- Observed 404 for application page
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/applications/meta/xactions/PhabricatorApplicationUninstallTransaction.php | ||
---|---|---|
61–64 | You should spell out two full separate variations of this for pht(). In some languages, the verb or noun or phrasing depend on the actor's gender or the subject's gender, so the proper phrasing may be:
...or:
In any case, even for languages without these rules, the words "uninstalled" and "installed" can't be translated at all right now. (See also Internationalization.) |
Yep, looks good to me.
src/applications/meta/xactions/PhabricatorApplicationUninstallTransaction.php | ||
---|---|---|
30 | It isn't terribly important, but this should probably be applyExternalEffects() instead. The distinction is that we do: begin_transaction(); internal_effects(); end_transaction(); external_effects(); So the theory is that "internal" effects apply to the object being edited and are covered by the transaction, and "external" effects apply to other objects. In practice, the entire thing is now always wrapped in another transaction (since T13054) and there's probably no real difference nowadays. We should perhaps combine the phases at some point. | |
57 | Super minor, but I think we use "this <thing>", not "the <thing>" elsewhere, at least in English. For example, "alice claimed this task." in Maniphest, not "alice claimed the task." | |
69 | These might read slightly more cleanly as "%s uninstalled %s.", i.e. "alice uninstalled Maniphest." vs "alice uninstalled the Maniphest application." I think we usually just say "Maniphest" elsewhere, although maybe it's mostly documentation/discussion rather than the actual software. That's might be a little ambiguous at first glance for applications like Config or System, but you usually can't uninstall those anyway. Not sure if there there are other cases where this would be confusing. |