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.

Sign up for GitHub

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

Jump to bottom

Add applied control duplication #940

Merged
merged 30 commits into from
Dec 4, 2024
Merged

Add applied control duplication #940

merged 30 commits into from
Dec 4, 2024

Conversation

monsieurswag
Copy link
Contributor

No description provided.

@Mohamed-Hacene Mohamed-Hacene self-requested a review October 15, 2024 09:13
@monsieurswag monsieurswag marked this pull request as draft October 15, 2024 09:14
Mohamed-Hacene
Mohamed-Hacene requested changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@Mohamed-Hacene Mohamed-Hacene left a comment •
edited by monsieurswag
Loading

Choose a reason for hiding this comment

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

  • Use a slot for duplication button inside the DetailView
  • Add a checkbox to bring the evidence if needed

@monsieurswag
Copy link
Contributor Author

We don't need a to use a slot now, the code can be easily adapted to add duplication for other objects.
The checkbox to bring the evidences has been added, check its behavior in the backend (some evidences are just linked while other are duplicated depending on their folder)

@monsieurswag monsieurswag force-pushed the CA-503-Duplicate-applied-controls branch 2 times, most recently from 4db9c47 to f710de5 Compare October 21, 2024 16:08
@monsieurswag monsieurswag marked this pull request as ready for review October 21, 2024 16:09
@monsieurswag monsieurswag marked this pull request as draft October 21, 2024 16:23
@monsieurswag monsieurswag marked this pull request as ready for review October 21, 2024 23:27
@monsieurswag monsieurswag marked this pull request as draft October 22, 2024 08:42
@monsieurswag monsieurswag force-pushed the CA-503-Duplicate-applied-controls branch from 6daaf06 to 6ccdfbe Compare October 23, 2024 09:53
@monsieurswag monsieurswag marked this pull request as ready for review October 23, 2024 12:43
nas-tabchiche
nas-tabchiche reviewed Nov 18, 2024
View reviewed changes
backend/core/views.py Outdated
)
duplicate_applied_control.save()

return Response({"results": "applied control duplicated"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's return the actual controls as JSON

Mohamed-Hacene
Mohamed-Hacene previously requested changes Nov 18, 2024
View reviewed changes
Copy link
Collaborator

@Mohamed-Hacene Mohamed-Hacene left a comment

Choose a reason for hiding this comment

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

  • Field has_evidence (and eta_missed by the way) should be excluded. We cannot add evidence from the applied control anymore.
    image
  • The checkbox has unrelated text with this PR
    image

@Mohamed-Hacene Mohamed-Hacene marked this pull request as draft November 18, 2024 11:23
@monsieurswag
Fix non-working duplicate modal and remove 2 useless values from the …
2cd22e5 
…AppliedControlReadSerializer

Backend formatter
@monsieurswag monsieurswag force-pushed the CA-503-Duplicate-applied-controls branch from 1237413 to 2cd22e5 Compare November 27, 2024 00:39
@monsieurswag monsieurswag marked this pull request as ready for review December 3, 2024 09:29
@ab-smith
Copy link
Contributor

ab-smith commented Dec 3, 2024

Pensez au squash au moment du merge

@monsieurswag monsieurswag force-pushed the CA-503-Duplicate-applied-controls branch from 46c272a to 9a555ff Compare December 4, 2024 17:33
eric-intuitem
eric-intuitem approved these changes Dec 4, 2024
View reviewed changes
Copy link
Collaborator

@eric-intuitem eric-intuitem left a comment

Choose a reason for hiding this comment

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

LGTM

@eric-intuitem eric-intuitem self-requested a review December 4, 2024 19:13
eric-intuitem
eric-intuitem approved these changes Dec 4, 2024
View reviewed changes
Copy link
Collaborator

@eric-intuitem eric-intuitem left a comment

Choose a reason for hiding this comment

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

LGTM

@eric-intuitem eric-intuitem merged commit 6a47df6 into main Dec 4, 2024
20 checks passed
@eric-intuitem eric-intuitem deleted the CA-503-Duplicate-applied-controls branch December 4, 2024 19:15
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eric-intuitem eric-intuitem eric-intuitem approved these changes

@nas-tabchiche nas-tabchiche Awaiting requested review from nas-tabchiche

@Mohamed-Hacene Mohamed-Hacene Awaiting requested review from Mohamed-Hacene

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@monsieurswag @ab-smith @nas-tabchiche @eric-intuitem @Mohamed-Hacene