Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed openByDefault to use filter #571

Merged
merged 2 commits into from
Oct 21, 2022
Merged

Conversation

bjosttveit
Copy link
Member

@bjosttveit bjosttveit commented Oct 20, 2022

Description

Open by default "first" and "last" is also broken by filters. In order to fix this the saga doing the open by default also needs to compute the filteredIndexList like the GroupContainer does. This function has been moved out of the component so that both the saga and the GroupContainer can use the same implementation.

I tried to make a general function, but the existing implementation did not make complete sense to me. It did not seem like it would work as documented, but by trying to reimplement it differently it broke some cypress tests.
The issues related to filters are documented here: #339 (comment)

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@bjosttveit bjosttveit linked an issue Oct 20, 2022 that may be closed by this pull request
@sonarcloud
Copy link

sonarcloud bot commented Oct 21, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

52.4% 52.4% Coverage
0.0% 0.0% Duplication

@bjosttveit bjosttveit marked this pull request as ready for review October 21, 2022 10:28
@bjosttveit bjosttveit assigned Magnusrm and unassigned Magnusrm Oct 21, 2022
@bjosttveit bjosttveit merged commit be15afc into main Oct 21, 2022
@bjosttveit bjosttveit deleted the fix/open-by-default-filter branch October 21, 2022 10:33
@bjosttveit
Copy link
Member Author

For future reference, here is the "more correct" implementation of getRepeatingGroupFilteredIndices:

/**
* Returns a list of remaining repeating group element indices after all filters are applied. Returns undefined if no filters are present.
* @param repeatingGroupIndex IRepeatingGroup.index for the repeating group.
* @param formData IFormData
* @param filter IGroupEditProperties.filter or undefined.
* @returns a list of indices for repeating group elements after applying filters, or null if no filters are provided.
*/
export function getRepeatingGroupFilteredIndices(
repeatingGroupIndex: number,
formData: IFormData,
filter?: IGroupFilter[],
): number[] | null {
if (filter && filter.length > 0) {
let filteredIndicies = Array.from(Array(repeatingGroupIndex + 1).keys());
filter.forEach((rule) => {
const formDataKeys: string[] = Object.keys(formData);
if (formDataKeys && formDataKeys.length > 0) {
const matchingSet = new Set<number>();
formDataKeys
.filter((key) => {
const keyWithoutIndex = key.replaceAll(/\[\d*\]/g, '');
return keyWithoutIndex === rule.key && formData[key] === rule.value;
})
.forEach((key) => {
const match = key.match(/\[(\d*)\]/g);
const currentIndex = match[match.length - 1];
const matchingIndex = parseInt(
currentIndex.substring(1, currentIndex.indexOf(']')),
10,
);
matchingSet.add(matchingIndex);
});
filteredIndicies = filteredIndicies.filter((index) =>
matchingSet.has(index),
);
return Array.from(matchingSet);
}
});
return filteredIndicies;
}
return null;
}

@olemartinorg olemartinorg added the kind/bug Something isn't working label Nov 15, 2022
@olemartinorg olemartinorg mentioned this pull request Feb 23, 2023
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openByDefault = "first" does not work with filters
3 participants