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

refactor: oAuth proxy through HTTP kernel #2969

Merged
merged 5 commits into from
Sep 2, 2019

Conversation

alirezamirsepassi
Copy link

@alirezamirsepassi alirezamirsepassi commented Sep 1, 2019

First of all thanks so much for taking the time to open a pull request and help the project. It's because of people like you that we love working on this project.

Please read the list below. Feel free to delete this text after but we need you to read it so we make sure that the project is consistent and remains of quality.

Checklist

Before submitting the PR

  • Read the CONTRIBUTING document before submitting your PR.
  • If the PR is related to an issue or fix one, don't forget to indicate it.
  • Indicate [wip] in the title of the PR it is is not final yet. Remove [wip] when ready. Otherwise the PR will be considered complete and rejected if it's not working.

General checks

  • Make sure that the change you propose is the smallest possible.
  • The name of the PR should follow the conventional commits guideline that the project follows.

Front-end changes

  • If you change the UI, make sure to ask repositories administrators first about your changes by pinging @djaiss or @asbiin in this PR.
  • Screenshots are included if the PR changes the UI.
  • Front-end tests have been written with Cypress.

Backend/models changes

  • The API has been updated.
  • API's documentation has been added by submitting a pull request in the marketing website repository.
  • Tests have been added for the new code.
  • If you change a model, make sure the FakeContentTableSeeder is updated. We need seeders to develop locally and generate fake data.

If the code changes the SQL schema

  • Make sure exporting account data as SQL is still working.
  • Make sure your changes do not break importing data with vCard and .csv files.
  • Make sure account reset and deletion still work.

Other tasks

  • CHANGELOG entry added, if necessary, under UNRELEASED.
  • CONTRIBUTORS entry added, if necessary.
  • If it's relevant and worth mentioning, create a changelog entry for this change. The changelog entry will appear inside the UI for all users to see. To know if your change is worth the creation of a changelog entry, read the documentation.
  • Don't forget to ask for a free account on https://monicahq.com as anyone who contributes can request a free account.

@CLAassistant
Copy link

CLAassistant commented Sep 1, 2019

CLA assistant check
All committers have signed the CLA.

@alirezamirsepassi alirezamirsepassi changed the title Handling proxy request through application kernel Refactor proxy request with application kernel Sep 1, 2019
@alirezamirsepassi alirezamirsepassi changed the title Refactor proxy request with application kernel refactor: oAuth proxy through HTTP kernel Sep 1, 2019
@alirezamirsepassi
Copy link
Author

This way your application can work with a single thread, also a more convenient way to do subrequests.

Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

Great job! Thank you.
See my comment below, and don't forget to apply StyleCI patch please.

app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Api/Auth/OAuthController.php Outdated Show resolved Hide resolved
@asbiin
Copy link
Member

asbiin commented Sep 2, 2019

Great!
After fixing the latest suggestions, you may push the commit to let styleCI run again. Then you can download a raw diff from StyleCI here:
image

and apply this diff with: patch -p1 < file.patch

@alirezamirsepassi
Copy link
Author

All done. Thank you for helping.

@asbiin asbiin merged commit 5c64932 into monicahq:master Sep 2, 2019
@asbiin
Copy link
Member

asbiin commented Sep 2, 2019

@alirezamirsepassi Thank you for your help!

@github-actions
Copy link

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2021
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