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

[spaceship] add update method to ConnectAPI::User #20956

Merged
merged 5 commits into from Jul 6, 2023

Conversation

nekrich
Copy link
Contributor

@nekrich nekrich commented Dec 29, 2022

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

We want to have the ability to change user roles and app access. And for that, we need Modify a User Account
API
implemented.

Description

  • Add a new request to update user. We can modify the User instance and just execute update or describe changes using parameters.
  • Fix UserInvitation.create docs.
  • Remove relationships for User.update & UserInvitation.requests if all apps are allowed to reduce the number of API errors.

Testing Steps

users = Spaceship::ConnectAPI::User.all
users.first.update(provisioning_allowed: false, roles: ['SALES'])

last_user = users.last
last_user.provisioning_allowed = false
last_user.roles = ['SALES', 'DEVELOPER']
last_user.update

@nekrich
Copy link
Contributor Author

nekrich commented Feb 15, 2023

@revolter , sorry for the ping, but if there is something I can improve?

@nekrich
Copy link
Contributor Author

nekrich commented Feb 23, 2023

@joshdholtz , do I have any chances you review this PR🙏 ?

@nekrich
Copy link
Contributor Author

nekrich commented Mar 1, 2023

@ainame , @rogerluan , any kind of feedback is appreciated 🙏 .

Copy link
Member

@rogerluan rogerluan left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Left some questions though 🙏

Thank you for this contribution @nekrich !

Copy link
Collaborator

@revolter revolter left a comment

Choose a reason for hiding this comment

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

Unfortunately, I don't have experience with this part of the app, so I can't say much (except some nitpicking) 😞

All the changes do make sense to me, though, and I trust that you actually tested them 💪🏻

Also, it doesn't look like it could break anything, but I'm not sure, so I'd wait for @joshdholtz to take a look too 😟

spaceship/spec/connect_api/users/user_client_spec.rb Outdated Show resolved Hide resolved
spaceship/spec/connect_api/users/user_client_spec.rb Outdated Show resolved Hide resolved
Co-authored-by: Iulian Onofrei <5748627+revolter@users.noreply.github.com>
@nekrich
Copy link
Contributor Author

nekrich commented Apr 11, 2023

Any chances of merging this?

@ainame ainame requested a review from joshdholtz April 11, 2023 13:23
@revolter
Copy link
Collaborator

revolter commented Jul 6, 2023

I took another look, and, again, it doesn't look like it could break existing logic, it rather adds new logic, so I will merge this.

@revolter revolter merged commit 18f7716 into fastlane:master Jul 6, 2023
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.

None yet

3 participants