Skip to content

Include a projects children in an alert rules limitations - #2013

Merged
nscuro merged 10 commits into
DependencyTrack:masterfrom
rbt-mm:master-limit-alert-rules-to-project-including-all-existing-or-future-versions
Oct 18, 2022
Merged

Include a projects children in an alert rules limitations#2013
nscuro merged 10 commits into
DependencyTrack:masterfrom
rbt-mm:master-limit-alert-rules-to-project-including-all-existing-or-future-versions

Conversation

@rbt-mm

@rbt-mm rbt-mm commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

#1608

Currently, when creating a limited alert rule you need to select every single version of a project. This does not work well for projects which create new versions regularly.
By automatically including every active child of a project to a notification rule, it is simpler to limit all existing or every future version of a project to a notification rule.

Frontend PR

Signed-off-by: RBickert rbt@mm-software.com

rbt-mm and others added 5 commits October 5, 2022 10:43
Currently, when creating a limited alert rule you need to select every
single version of a project. This does not work well for projects which
create new versions regularly.
By automatically subscribing every child of a project to a notification
rule, it is simpler to limit all existing or every future version of a
project to a notification rule.

Signed-off-by: RBickert <rbt@mm-software.com>
Added tests in `NotificationRouterTest.java` which cover the cases of a
subscribed parent project with an affected child and an inactive
affected child.

Signed-off-by: RBickert <rbt@mm-software.com>
Signed-off-by: RBickert <rbt@mm-software.com>
Signed-off-by: RBickert <rbt@mm-software.com>
…cluding-all-existing-or-future-versions

Limit alert rules to project including all existing or future versions
@sonatype-lift

sonatype-lift Bot commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

⚠️ 14 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

@nscuro nscuro added this to the 4.7 milestone Oct 9, 2022
@rbt-mm

rbt-mm commented Oct 10, 2022

Copy link
Copy Markdown
Contributor Author

Hey @nscuro, I'm not quite sure why this build failed. The workflow runs successfully in our repository and I don't think that I've changed anything which would throw a MojoExecutionException.
Do you know if something is wrong with my changed code or is it just a build fail? And if so, how can I restart the workflows to pass the checks?

@nscuro

nscuro commented Oct 10, 2022

Copy link
Copy Markdown
Member

No worries @rbt-mm, it's just GitHub being GitHub again. When trying to download the frontend ZIP, the GitHub API returned a 503, causing the build to fail.

I don't think contributors can manually trigger workflows, but you could always contact us maintainers (as you did), or push an empty commit to trigger another workflow execution.

@nscuro nscuro self-assigned this Oct 12, 2022

@nscuro nscuro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR @rbt-mm!

Overall this looks good to me. But I was wondering whether this should be an option that users can opt-in to / opt-out of? Just because there is a hierarchy, it doesn't necessarily mean I'm interested in all notifications affecting child projects.

@rkg-mm

rkg-mm commented Oct 13, 2022

Copy link
Copy Markdown
Contributor

Thanks for the PR @rbt-mm!

Overall this looks good to me. But I was wondering whether this should be an option that users can opt-in to / opt-out of? Just because there is a hierarchy, it doesn't necessarily mean I'm interested in all notifications affecting child projects.

Yes that's something I was also thinking about, but for simplicity decided against it for the first attempt. But the question is:

  1. Would this be a setting per notification rule, for all projects in that rule? (simple, add a checkbox and an additional field in the rule)
  2. Or a setting per project in each notification rule (requires a field in the mapping table, not sure if datanucleus supports that without big changes, and also would have to refactor the project List to show an additional checkbox per project, plus APIs to update etc.)

If you think option 1) is enough we can easily do that, for 2) we would have to figure out how and will take some more time.

@nscuro

nscuro commented Oct 13, 2022

Copy link
Copy Markdown
Member

I think option 1 should suffice.

rbt-mm and others added 5 commits October 18, 2022 12:44
…roject-including-all-existing-or-future-versions
Added new attribute to 'NotificationRule' which optionally includes
every active child of the subscribed projects in the notification rule.

Signed-off-by: RBickert <rbt@mm-software.com>
…o-project-including-all-existing-or-future-versions
…ing-or-future-versions' into rbt-limit-alert-rules-to-project-including-all-existing-or-future-versions
…cluding-all-existing-or-future-versions

Optionally include a projects children in an alert rules limitations
@rbt-mm
rbt-mm requested a review from nscuro October 18, 2022 12:20
@nscuro
nscuro merged commit 0963d65 into DependencyTrack:master Oct 18, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants