[MGNLUI-7248] AbstractAvailabilityRule: faulty check of multiple items when negate is true Created: 16/Jun/22  Updated: 07/Sep/22  Resolved: 07/Sep/22

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 6.2.19
Fix Version/s: 6.3.0, 6.2.24

Type: Bug Priority: Neutral
Reporter: Philipp Guettler Assignee: Adam Siska
Resolution: Fixed Votes: 0
Labels: quickwin
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLUI-7490 Create PR with code from ticket descr... Sub-task Completed Adam Siska  
MGNLUI-7491 Code review Sub-task Completed Rabie Hayoun  
MGNLUI-7492 Preint QA Sub-task Completed Roman Kovařík  
MGNLUI-7493 QA Sub-task Completed David Lopez  
MGNLUI-7494 Backport to 6.2 Sub-task Completed Adam Siska  
MGNLUI-7503 Integrate Sub-task Completed Adam Siska  
Template:
Patch included:
Yes
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[X]* Steps to reproduce, expected, and actual results filled
[X]* Affected version filled
Date of First Response:
Epic Link: Nucleus Quality Maintenance
Sprint: Nucleus 18
Story Points: 2
Team: Nucleus

 Description   

The logic implemented in info.magnolia.ui.availability.rule.AbstractAvailabilityRule#isAvailable is wrong, when testing multiple items and AvailabilityRuleDefinition#getNegate returns true. Consider the following unit test, the last assertion should not fail.

@Test
public void testAvailabilityRuleForMultipleItems() {
    ConfiguredAvailabilityDefinition availabilityDefinition = new ConfiguredAvailabilityDefinition();
    availabilityDefinition.setNodeTypes(List.of(NodeTypes.Page.NAME));

    JcrNodeTypeRuleDefinition isPageDefinition = new JcrNodeTypeRuleDefinition();
    JcrNodeTypeRule isPage = new JcrNodeTypeRule(availabilityDefinition, isPageDefinition);

    JcrNodeTypeRuleDefinition notPageDefinition = new JcrNodeTypeRuleDefinition();
    notPageDefinition.setNegate(true);
    JcrNodeTypeRule isNotPage = new JcrNodeTypeRule(availabilityDefinition, notPageDefinition);

    MockNode page = new MockNode("page", NodeTypes.Page.NAME);
    Collection<Node> listOfPages = List.of(page, page);
    Assert.assertTrue(isPage.isAvailable(listOfPages));
    Assert.assertFalse(isNotPage.isAvailable(listOfPages)); 

    MockNode area = new MockNode("area", NodeTypes.Component.NAME);
    Collection<Node> listOfAreas = List.of(area, area);
    Assert.assertFalse(isPage.isAvailable(listOfAreas));
    Assert.assertTrue(isNotPage.isAvailable(listOfAreas));;

    Collection<Node> listOfPagesAndAreas = List.of(page, area);
    Assert.assertFalse(isPage.isAvailable(listOfPagesAndAreas));
    Assert.assertFalse(isNotPage.isAvailable(listOfPagesAndAreas)); // FAILS, but isAvailable should return false
} 


Consider the following implementation as a possible solution

info.magnolia.ui.availability.rule.AbstractAvailabilityRule
@Override
public boolean isAvailable(Collection<?> items) {
    if (shouldByPass(items)) {
        return true;
    }
    
    for (Object item : items) {
        if (getRuleDefinition().getNegate() == isAvailableForItem(item)) {
            return false;
        }
    }

    return true;
} 


 Comments   
Comment by Adam Siska [ 25/Aug/22 ]

Discovery:

  • solution from ticket description looks feasible
  • regression test could be used as it is or as a draft to provide test for AbstractAvailabilityRule
Generated at Mon Feb 12 09:44:29 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.