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

[MM-36742] Fix immediately expiring request context #452

Merged
merged 2 commits into from
Jul 8, 2021

Conversation

hanzei
Copy link
Contributor

@hanzei hanzei commented Jun 30, 2021

Summary

The caller of createContext needs to cancel it instead of createContext itself.

Ticket Link

https://mattermost.atlassian.net/browse/MM-36742
Fixes #451

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Jun 30, 2021
@hanzei hanzei added this to the v2.1.0 milestone Jun 30, 2021
@hanzei hanzei requested review from larkox and mickmister June 30, 2021 10:34
@mickmister mickmister removed the 2: Dev Review Requires review by a core committer label Jun 30, 2021
@mickmister
Copy link
Member

@hanzei Does this need QA review? What would be tested?

@hanzei
Copy link
Contributor Author

hanzei commented Jul 1, 2021

@DHaussermann Could you please test if the RHS infos get correctly loaded and picking an repo in the create issue modal works?

@hanzei hanzei requested a review from DHaussermann July 1, 2021 11:13
@DHaussermann
Copy link

DHaussermann commented Jul 5, 2021

@hanzei This resolves the list of repos not loading and it is now possible to create and attach.
However I still see a remaining issue. The list of assignees is still not populating and I get 500 responses still for this.
image
image
I have reproduced this on multiple public and private repos. Is this solution maybe incomplete or would this require a saparate PR?

@hanzei
Copy link
Contributor Author

hanzei commented Jul 7, 2021

@DHaussermann Good catch. Fixed that in 93a453e. Please take another look.

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed.

  • Issue with 500 responses for assignee and Repo are resolved
  • Tested on desktop and browser
  • Tested for create and attach
  • Tested on public and private repos

LGTM!

Huge thanks @hanzei! Please merge, this will unblock 2 other PRs waiting for testing.

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jul 7, 2021
@hanzei hanzei merged commit 07f04cd into master Jul 8, 2021
@hanzei hanzei deleted the MM-36742_fix-cancel branch July 8, 2021 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub - Unable to fetch repos (master branch)
4 participants