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

[15.0][MIG] auth_jwt,auth_jwt_demo: pre-migrate to V15 #320

Merged
merged 30 commits into from
Jan 19, 2022

Conversation

yankinmax
Copy link
Contributor

@yankinmax yankinmax commented Dec 29, 2021

This PR is needed to port code from V14 to V15.
Modules will be migrated here:

@yankinmax
Copy link
Contributor Author

@sbidoul @simahawk
this PR is related to this suggestion:
#316 (comment)

@simahawk
Copy link
Contributor

@yankinmax you have to exclude these modules from pre-commit conf

@sbidoul
Copy link
Member

sbidoul commented Jan 17, 2022

Ok, so if I understand this comment correctly, this PR should also mark auth_jwt_demo as installable=False ?

@yankinmax
Copy link
Contributor Author

Ok, so if I understand this comment correctly, this PR should also mark auth_jwt_demo as installable=False ?

I suppose yes. If I'm doing something wrong pls correct me.

@simahawk
Copy link
Contributor

@yankinmax fix pre-commit pls

@yankinmax yankinmax force-pushed the 15.0-pre-mig-auth_jwt,auth_jwt_demo branch 2 times, most recently from 44e4d62 to 179d372 Compare January 19, 2022 13:09
@@ -5,7 +5,7 @@
"name": "Auth JWT",
"summary": """
JWT bearer token authentication.""",
"version": "14.0.1.2.0",
"version": "15.0.1.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Please do not change the version while it is not migrated

@@ -114,7 +114,7 @@ def _decode(self, token):
header = jwt.get_unverified_header(token)
except Exception as e:
_logger.info("Invalid token: %s", e)
raise UnauthorizedInvalidToken()
raise UnauthorizedInvalidToken() from e
Copy link
Member

Choose a reason for hiding this comment

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

This change should not be here yet, as you are just setting the modules as installable=False

@@ -5,12 +5,13 @@
"name": "Auth JWT Test",
"summary": """
Test/demo module for auth_jwt.""",
"version": "14.0.1.2.0",
"version": "15.0.1.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Don't change the version yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was done to avoid pre-commit issues

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Please commit the change to the pre-commit config.
Also, make sure the two commits that set the modules to installable=False are the last in the list and all the other come from git am.

@yankinmax
Copy link
Contributor Author

@sbidoul I'll rebase right now.

  1. .pre-commit-config.yaml will leave as after pre-commit run -a
  2. last two commits with installable=False
  3. version as it is.

sbidoul and others added 11 commits January 19, 2022 17:35
This method is useful for public endpoints that need
to work for anonymous user, but can be enhanced when
an authenticated user is know.

A typical use case is a "add to cart" enpoint that can
work for anonymous users, but can be enhanced by
binding the cart to a known customer when the authenticated
user is known.
@yankinmax yankinmax force-pushed the 15.0-pre-mig-auth_jwt,auth_jwt_demo branch from 179d372 to 35b0e68 Compare January 19, 2022 15:36
@sbidoul
Copy link
Member

sbidoul commented Jan 19, 2022

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-320-by-sbidoul-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ff5dea4 into OCA:15.0 Jan 19, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 3816df3. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants