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

feat: add keyEmbeddedDashboardsEnabled [DHIS2-18472] #1409

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomzemp
Copy link
Member

@tomzemp tomzemp commented Nov 28, 2024

This PR:

  • adds the new setting keyEmbeddedDashboardsEnabled under Analytics section (see ticket: https://dhis2.atlassian.net/browse/DHIS2-18472)
  • adds version control on the settings definitions (this app became continuous release for v41, so previously we have not had to toggle on/off individual settings by version)
  • fixes some filtering logic for the search at the category level. For this, I've moved some of the logic for categories filtering by version. We had added a maximumApiVersion on the category level to remove oAuth2 after v41, but I noticed when adding the setting-level versioning that the search functionality was not working properly as we also have to filter categories + settings in the search space

I think the approach here is okay. It would be more efficient to just do the filtering once and then set the results in state, or memoize the results rather than performing this filtering both when building the search space and when switching categories. However, I think the filtering operation is fairly trivial as a calculation, and I don't want to refactor more (particularly I think doing so would require reviewing the logic of the search, which seems convoluted).

@tomzemp tomzemp requested a review from a team November 28, 2024 09:31
@dhis2-bot
Copy link
Contributor

🚀 Deployed on https://pr-1409--dhis2-settings.netlify.app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants