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

Finding group enhancements #6053

Merged
merged 17 commits into from
Mar 31, 2022
Merged

Conversation

dsever
Copy link
Contributor

@dsever dsever commented Mar 22, 2022

  • This PR shows group real names names instead of Y and N.
  • removes 'Experimental notification'
  • and moves configuration (enable/disable) from config file to DB
    image
    image
    image

- use group names in the forms
- moving enable feature to database
@github-actions github-actions bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests labels Mar 22, 2022
@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Mar 22, 2022
@valentijnscholten valentijnscholten changed the title Finding groups Finding group enhancements Mar 22, 2022
dojo/models.py Outdated Show resolved Hide resolved
dojo/utils.py Outdated Show resolved Hide resolved
Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

I think this PR should either migrate the existing setting into the DB for existing installs and/or add something to the upgrade notes. Ideally if users have disabled finding groups, it should remain disabled after upgrading to the version that includes this PR.

@dsever
Copy link
Contributor Author

dsever commented Mar 23, 2022

I think this PR should either migrate the existing setting into the DB for existing installs and/or add something to the upgrade notes. Ideally if users have disabled finding groups, it should remain disabled after upgrading to the version that includes this PR.

This is true, will try to add it

@dsever
Copy link
Contributor Author

dsever commented Mar 23, 2022

@StefanFl @valentijnscholten Changes have been implemented

Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

Thanks. Experience has shown it's better to separate schema migrations and data migrations. Data migrations sometimes fail and having them in a separate migration allows for easy retry without having to comment out any schema parts that were successful. Could you split them up?

@dsever
Copy link
Contributor Author

dsever commented Mar 24, 2022

Thanks. Experience has shown it's better to separate schema migrations and data migrations. Data migrations sometimes fail and having them in a separate migration allows for easy retry without having to comment out any schema parts that were successful. Could you split them up?

@valentijnscholten, Done

@valentijnscholten valentijnscholten merged commit 8db578f into DefectDojo:dev Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Migration Adding a new migration file. Take care when merging. settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants