Skip to content

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

Merged
revolter merged 5 commits into
fastlane:masterfrom
nekrich:master
Jul 6, 2023
Merged

[spaceship] add update method to ConnectAPI::User#20956
revolter merged 5 commits into
fastlane:masterfrom
nekrich:master

Conversation

@nekrich

@nekrich nekrich commented Dec 29, 2022

Copy link
Copy Markdown
Contributor

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

nekrich commented Feb 15, 2023

Copy link
Copy Markdown
Contributor Author

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

@nekrich

nekrich commented Feb 23, 2023

Copy link
Copy Markdown
Contributor Author

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

@nekrich

nekrich commented Mar 1, 2023

Copy link
Copy Markdown
Contributor Author

@ainame , @rogerluan , any kind of feedback is appreciated 馃檹 .

@rogerluan rogerluan left a comment

Copy link
Copy Markdown
Member

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 !

Comment thread spaceship/lib/spaceship/connect_api/models/user_invitation.rb
Comment thread spaceship/lib/spaceship/connect_api/models/user.rb Outdated
Comment thread spaceship/lib/spaceship/connect_api/models/user.rb Outdated
Comment thread spaceship/lib/spaceship/connect_api/models/user.rb
Comment thread spaceship/lib/spaceship/connect_api/users/users.rb

@revolter revolter left a comment

Copy link
Copy Markdown
Collaborator

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 馃槦

Comment thread spaceship/spec/connect_api/users/user_client_spec.rb Outdated
Comment thread spaceship/spec/connect_api/users/user_client_spec.rb Outdated
Co-authored-by: Iulian Onofrei <5748627+revolter@users.noreply.github.com>
@nekrich

nekrich commented Apr 11, 2023

Copy link
Copy Markdown
Contributor Author

Any chances of merging this?

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

revolter commented Jul 6, 2023

Copy link
Copy Markdown
Collaborator

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
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants