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

[CONSVC-1729] feat: introduce approval for partners #39

Merged
merged 2 commits into from
Apr 28, 2022

Conversation

tiftran
Copy link
Contributor

@tiftran tiftran commented Apr 28, 2022

decided that if a partner setting has changed, you are unable to create a snapshot with that partner unless it has been approved.
closes #23

@tiftran tiftran force-pushed the 2-ppl-partner-sign-off branch 2 times, most recently from c3c3fa1 to b1b25a1 Compare April 28, 2022 07:25
@tiftran tiftran changed the title feat: introduce approval for partners [CONSVC-1729] feat: introduce approval for partners Apr 28, 2022
@tiftran tiftran requested a review from a team April 28, 2022 07:42

def clean(self):
if (
self.settings_type.is_active is False
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
self.settings_type.is_active is False
!self.settings_type.is_active

Copy link
Contributor Author

Choose a reason for hiding this comment

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

! self.settings_type.is_active isn't valid python will be changing it to not self.settings_type.is_active

Copy link
Contributor

Choose a reason for hiding this comment

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

Doh, my Python is getting Rusty 😂.

contile/admin.py Outdated
messages.info(request, f"Partner: {partner.name} has been approved")
else:
messages.error(
request, "Approver is the same user that updated the partner"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: let's also point out how this should be done, such as "This change can't be approved by the same editor, please get another reviewer for approval".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good

@tiftran tiftran requested a review from ncloudioj April 28, 2022 18:08
Copy link
Contributor

@ncloudioj ncloudioj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tiftran tiftran merged commit 68bddf6 into main Apr 28, 2022
tiftran added a commit that referenced this pull request Aug 3, 2022
* feat: introduce approval for partners

* feedback changes
@tiftran tiftran deleted the 2-ppl-partner-sign-off branch June 8, 2023 17:13
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.

Add method to enforce two person sign off in partner model
2 participants