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

feat: Add role API key (BE) #3346

Merged
merged 2 commits into from
Feb 14, 2024
Merged

feat: Add role API key (BE) #3346

merged 2 commits into from
Feb 14, 2024

Conversation

novakzaballa
Copy link
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

How did you test this code?

@vercel Vercel
Copy link

vercel bot commented Jan 29, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 2:05pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 2:05pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 2:05pm

@github-actions github-actions bot added the api Issue related to the REST API label Jan 29, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 29, 2024 •
edited
Loading

Uffizzi Preview deployment-45329 was deleted.

@novakzaballa novakzaballa mentioned this pull request Jan 29, 2024
Merged
4 tasks
matthewelwell
matthewelwell requested changes Jan 29, 2024
View reviewed changes
api/api_keys/models.py Outdated
@@ -18,3 +20,12 @@ class MasterAPIKey(AbstractAPIKey, SoftDeleteObject):

objects = MasterAPIKeyManager()
is_admin = models.BooleanField(default=True)

@hook(BEFORE_UPDATE, when="is_admin", was=False, is_now=True)
def delete_role_api_keys(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a test for this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test is here: https://github.com/Flagsmith/flagsmith-rbac/pull/19

Copy link
Contributor

@matthewelwell matthewelwell Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In that case, I think we should move all of this code to a signal in the RBAC repository.

See here for an example:

https://github.com/Flagsmith/flagsmith-ldap/blob/main/flagsmith_ldap/signals.py
https://github.com/Flagsmith/flagsmith-ldap/blob/main/flagsmith_ldap/apps.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to RBAC

@novakzaballa novakzaballa changed the title feat: Add role API key in UI (BE) feat: Add role API key (BE) Feb 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (351232f) 95.95% compared to head (71e2437) 95.91%.
Report is 57 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3346      +/-   ##
==========================================
- Coverage   95.95%   95.91%   -0.04%     
==========================================
  Files        1067     1085      +18     
  Lines       32673    33677    +1004     
==========================================
+ Hits        31350    32302     +952     
- Misses       1323     1375      +52

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@novakzaballa novakzaballa added this pull request to the merge queue Feb 14, 2024
Merged via the queue into main with commit c60a145 Feb 14, 2024
21 checks passed
@novakzaballa novakzaballa deleted the feat/add-role-api-key-in-ui-be branch February 14, 2024 17:06
@dabeeeenster dabeeeenster mentioned this pull request Feb 14, 2024
Merged
@dabeeeenster dabeeeenster mentioned this pull request Apr 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewelwell matthewelwell matthewelwell approved these changes

Assignees
No one assigned
Labels
api Issue related to the REST API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@novakzaballa @codecov-commenter @matthewelwell