Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

[MM-12738] Migrate OAuth app actions from user_actions.jsx and admin_… #2387

Merged
merged 2 commits into from
Feb 28, 2019

Conversation

mmaksitaliev
Copy link
Contributor

@mmaksitaliev mmaksitaliev commented Feb 17, 2019

Summary

Migrated actions:

  • allowOAuth2
  • getOAuthAppInfo
  • getAuthorizedApps
  • deauthorizeOAuthApp

Updated corresponding components

Ticket Link

MM-12738

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)
  • Touches critical sections of the codebase (OAuth related actions)

@jasonblais jasonblais added 2: Dev Review Requires review by a core commiter Hackfest labels Feb 17, 2019
@jwilander jwilander removed their request for review February 20, 2019 21:55
@saturninoabril
Copy link
Member

Requesting review from @sudheerDev as he's given more thoughts about this.

@saturninoabril saturninoabril requested review from sudheerDev and removed request for saturninoabril February 21, 2019 13:55
@mmaksitaliev
Copy link
Contributor Author

PR is updated, have a look please

Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Thanks for moving that back out of redux. Looks good to me now.

@enahum enahum requested review from deanwhillier and removed request for sudheerDev February 25, 2019 18:20
Copy link
Contributor

@deanwhillier deanwhillier 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 @MirlanMaksv! Could you please merge master back into into your branch to bring it up to date so I can merge the PR? Thx!

@mmaksitaliev
Copy link
Contributor Author

@deanwhillier, done

@deanwhillier deanwhillier added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Feb 28, 2019
@deanwhillier deanwhillier merged commit 6e3b7b6 into mattermost:master Feb 28, 2019
@deanwhillier
Copy link
Contributor

Thats great, thanks again @MirlanMaksv!

@deanwhillier deanwhillier removed the 4: Reviews Complete All reviewers have approved the pull request label Feb 28, 2019
@mmaksitaliev
Copy link
Contributor Author

I made mistake in user_settings_security test, now it is polluting test output with a warning. Here is the PR with a fix.

I am sorry for troubles

@hmhealey hmhealey added this to the v5.10.0 milestone Mar 1, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Mar 19, 2019
stevepartridge pushed a commit to stevepartridge/mattermost-webapp that referenced this pull request Mar 30, 2019
@lindy65 lindy65 added the Tests/Not Needed Does not require new release tests label Apr 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Hackfest Tests/Not Needed Does not require new release tests
Projects
None yet
7 participants