[MGNLRES-185] Replace add hot-fix action with edit resource action Created: 06/Aug/15 Updated: 29/Mar/22 Resolved: 03/Sep/15 |
|
| Status: | Closed |
| Project: | Magnolia Resources Module |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4.2 |
| Type: | Story | Priority: | Neutral |
| Reporter: | Andreas Weder | Assignee: | Ilgun Ilgun |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | ux | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 7h | ||
| Original Estimate: | Not Specified | ||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Release notes required: |
Yes
|
| Date of First Response: | |
| Epic Link: | Phase out in-place templating app |
| Sprint: | Sprint 7 (Basel), Sprint (Basel) 8 |
| Story Points: | 3 |
| Team: |
| Description |
|
The idea here is that we remove "add hot-fix" and only offer "edit resource". More details here: https://wiki.magnolia-cms.com/display/UX/The+Resource+Files+app#TheResourcefilesapp-Editingaresourcefile Acceptance criteria
|
| Comments |
| Comment by Andreas Weder [ 07/Aug/15 ] |
|
Although this only covers the removal of "add hotfix", I suggest we do not ship this in a release without also at least renaming the "remove hotfix" action. Actually, this is covered here: https://jira.magnolia-cms.com/browse/MGNLRES-186 |
| Comment by Ilgun Ilgun [ 17/Aug/15 ] |
|
Changes can be found in feature/remove-hotfix-action/ |
| Comment by Aleksandr Pchelintcev [ 20/Aug/15 ] |
|
1) EditResourceFileAction - why not EditResourceAction? JavaDoc, consider this: * Action for editing a file within the Resources app. * <p> * This action will try to find the file for editing within {@value JcrResourceOrigin#RESOURCES_WORKSPACE} JCR workspace. * If no such file exist then a so-called 'hotfix' will be created first, which involves importing a resource from classpath or file system into the {@value JcrResourceOrigin#RESOURCES_WORKSPACE} JCR workspace. * By doing this, the newly imported resource takes precedence over the original one in the resources loading cascade. * It may then be edited, on a live instance, and it may eventually be published to a public instance as well. 2) Comment in the code:
// notify eventBus for changed content.
You're notifying not an event bus, but the subscribers of an event. Pls change accordingly or remove. 3)
static final String DETAIL_SUBAPP_NAME = "detail";
static final String HOTFIX_SUBAPP_NAME = "hotfix";
why do we still need two sub-apps? since we do hot-fixing on the fly - why not always using the 'detail' sub-app? Pls clarify. 4) How does this actually replace anything? If you had the previous stuff in JCR (say you're migrating 2.4.0 version) - it's gonna remain in there. Pls use the correct MVH tasks and add MVH test which actually reproduces the replacements of the properties and actions. register(DeltaBuilder.update("2.4.1", "") .addTask(new NodeExistsDelegateTask("Replacing hotfixAction with editResource action.", "/modules/resources-app/apps/resources", new BootstrapSingleResource("", "", "/mgnl-bootstrap/resources-app/config.modules.resources-app.apps.resources.xml")))); 5) Guess bootstrap file could lose the activationStatus related entries ('mgnl:activationStatus' and such) 6) We eliminate the 'Create hot fix' action but keep the 'Remove hot fix' one - how the new user's gonna find out what the hotfix is all about. 7) Translations: 8) Pls mind the formatting changes and such - try to not add unnecessary modifications, check the commits before pushing them. 9) Pls also check w/ Andreas if one time notification implies that it is shown once for each user |
| Comment by Ilgun Ilgun [ 21/Aug/15 ] |
|
1) Simply because EditResourceAction already exists, JavaDoc improved accordingly. Changes can be found at the same branch. |
| Comment by Aleksandr Pchelintcev [ 21/Aug/15 ] |
|
| Comment by Ilgun Ilgun [ 21/Aug/15 ] |
|
Pushed into the same branch. |
| Comment by Jan Haderka [ 01/Sep/15 ] |
|
Since the other action was already released we need to keep the class and deprecate it instead of just removing it w/o any notice or explanation so anyone using or struggling with old class can find the new one. |