[MGNLDAM-654] Assets: restore previous version throws exception Created: 18/May/16  Updated: 16/Jun/16  Resolved: 15/Jun/16

Status: Closed
Project: Magnolia DAM Module
Component/s: DAM App
Affects Version/s: 2.1.5
Fix Version/s: None

Type: Bug Priority: Neutral
Reporter: Sigurd Rolfes Assignee: Hieu Nguyen Duc
Resolution: Duplicate Votes: 0
Labels: support
Remaining Estimate: 0d
Time Spent: 4d 5h
Original Estimate: 4d
Environment:

5.3.15 Enterprise Edition, 5.4.7 Enterprise Edition, 5.5 Enterprise Edition.


Attachments: PNG File asset-restore.png    
Issue Links:
Cloners
Relates
relates to MAGNOLIA-6157 Allow to configure RestorePreviousVer... Closed
duplicate
duplicates MGNLDAM-606 Error restoring a deleted asset to it... Closed
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)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Saigon 48
Story Points: 5

 Description   

Deleting an asset and then trying to restore previous version an exception is thrown.



 Comments   
Comment by Hieu Nguyen Duc [ 08/Jun/16 ]
Explanation
  • RestorePreviousVersionCommand restores children of deleted node based on two criteria parentNodeTypeOnly and rule.
  • Asset app doesn't have parentNodeTypeOnly so it applies RestorePreviousVersionCommand#getDefaultRule
  • Unfortunately with this rule, filtering gets even jcr:content which is not versionable.
  • Getting previous version of jcr:content causes the exception.
There are two solutions
  1. Add parentNodeTypeOnly to RestoreItemPreviousVersionAction action.
  2. Add rule to RestoreItemPreviousVersionAction action.

mgeljic Please help me review it. Thanks.

Comment by Robert Šiška [ 15/Jun/16 ]

Hello.

The solution #1 is a good one, but simply setting parentNodeTypeOnly to the action would break restoring of folders again.
So, we should create two actions - one for assets and one for folders and set it only on the former.

I think that this solution is better than #2 which is unnecessarily complicated + plus all changes can stay in dam.

I'm firing up new PR from MGNLDAM-606 and closing this ticket as duplicated...

Comment by Hieu Nguyen Duc [ 16/Jun/16 ]

Wow, thanks for that.

Comment by Mikaël Geljić [ 16/Jun/16 ]

Okay, so first the jcr:content doesn't need to be a mix:versionable; it is copied correctly when versioning. So far so good.

Thanks rsiska for the folder insights again; however I'm not fond of duplicating the action either. This seems to me as much complicated as option #2, except you need actions, actionbar and availability configuration which are terribly verbose — and when this occurs in resources workspace, we're screwed and have to do it again.

What I don't understand is that we should be able to rely on the default rule properly: info.magnolia.commands.impl.RuleBasedCommand#getDefaultRule
It always has at least the mgnl:contentNode type, and adds the two mgnl:metaData and mgnl:resource. Again, all good when creating versions.

So isn't it the restore action not consuming this rule properly?

(And ideally, if our versioning were reading node-type definitions, we could just make sure to use the correct On-Parent-Version action for the jcr:content node, right in magnolia-dam-nodetypes.xml, but different story).

Generated at Mon Feb 12 05:01:57 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.