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

Create RPC delete collaborator #6489

Merged
merged 13 commits into from
Sep 1, 2023
Merged

Conversation

nicholaspcr
Copy link
Contributor

Summary

References #6314

Changes

  • Add deleteCollaborator for: [clients, gateways, organizations, applications]
  • Update tests to validate deleteCollaborator behavior

Testing

Update on unit tests and manual tests via the changes done on PRs that will be linked shortly.

Steps to reproduce and test the changes.

Since this will be merge only when all the subsequent PRs are approved, it means that the Console and CLI will be calling the new route. So in order to test just add and delete a collaborator to the entities mentioned in the changes section.

Nevertheless I will write some more detailed steps on the referenced issue.

Regressions

New rpc service and it does not change the behavior of the update route, there should not be any regressions.

Notes for Reviewers

Broke this into smaller PRs in order to avoid having huge PRs.

Didn't add anything on the CHANGELOG since this is mostly internal. It is probably worth adding that deletion of collaborators won't be able to be performed via the update route but that will only happen on a major release.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • The steps/process to test this feature are clearly explained including testing for regressions.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@nicholaspcr nicholaspcr added c/identity server This is related to the Identity Server technical debt Not necessarily broken, but could be done better/cleaner labels Aug 29, 2023
@nicholaspcr nicholaspcr added this to the v3.27.2 milestone Aug 29, 2023
@nicholaspcr nicholaspcr self-assigned this Aug 29, 2023
@nicholaspcr nicholaspcr force-pushed the issue/6314-rpc-delete-membership branch from 6195205 to e266ebe Compare August 31, 2023 00:20
Copy link
Contributor

@adriansmares adriansmares left a comment

Choose a reason for hiding this comment

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

LGTM in general.

pkg/identityserver/application_access_test.go Show resolved Hide resolved
@nicholaspcr nicholaspcr force-pushed the issue/6314-rpc-delete-membership branch from e266ebe to e60b0d7 Compare August 31, 2023 11:04
Copy link
Member

@johanstokking johanstokking left a comment

Choose a reason for hiding this comment

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

Skimmed over it; you can merge

@nicholaspcr nicholaspcr merged commit 4cd5556 into v3.27 Sep 1, 2023
15 of 16 checks passed
@nicholaspcr nicholaspcr deleted the issue/6314-rpc-delete-membership branch September 1, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/identity server This is related to the Identity Server technical debt Not necessarily broken, but could be done better/cleaner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants