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

fix: settings for scan types #511

Merged
merged 4 commits into from
Apr 18, 2024
Merged

fix: settings for scan types #511

merged 4 commits into from
Apr 18, 2024

Conversation

teodora-sandu
Copy link
Contributor

Description

At the moment when we change the scan type selection the Tree Nodes associated with them don't get re-enabled. This seems to be because the changes in the ScanTypesPanel don't actually get trickled up to SnykProjectSettingsConfigurable in order for the apply() method to run when isModified() returns true. The reason for this is that when the selections change the Dialog.isModified() function called in isScanTypeChanged in SnykSettingsDialog hasn't registered that there has been a change. We're now achieving that with the use of bindSelected.

We have some validation in place so that pre-apply we don't allow people to deselect all scan types. The new selections only get "persisted" globally once the apply() function gets called so the new action listener we added for each checkbox also needs to locally persist the selection changes in order for the validation to work.

Docs about these panels that led to this implementation:

There are two more panels that probably have this issue. The SeveritiesEnablementPanel and IssueViewOptionsPanel. Before I make those changes there I want to see if this approach is okay.

Checklist

  • Tests added and all succeed
  • Linted
  • CHANGELOG.md updated
  • README.md updated, if user-facing

@bastiandoetsch bastiandoetsch marked this pull request as ready for review April 16, 2024 11:24
@bastiandoetsch bastiandoetsch requested a review from a team as a code owner April 16, 2024 11:24
Copy link
Collaborator

@bastiandoetsch bastiandoetsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice finding, didn't know bindSelected before.

@teodora-sandu teodora-sandu force-pushed the fix/settings-scan-type branch 2 times, most recently from 5ffba4a to 6f1a558 Compare April 17, 2024 17:42
@teodora-sandu teodora-sandu force-pushed the fix/settings-scan-type branch from 6f1a558 to 229934b Compare April 18, 2024 09:27
@teodora-sandu teodora-sandu enabled auto-merge (squash) April 18, 2024 09:27
@teodora-sandu teodora-sandu merged commit 0460fe4 into master Apr 18, 2024
9 checks passed
@teodora-sandu teodora-sandu deleted the fix/settings-scan-type branch April 18, 2024 09:38
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