-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: Remove JWT_AUTH_REFRESH_COOKIE Setting depr190 #32647
Conversation
Thanks for the pull request, @Yagnesh1998! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
@e0d I think all checks have passed. |
@robrap and @dianakhuang I believe you were involved in scoping the issue that inspired this work. OSPR is very small. Can one of you review? |
@Yagnesh1998: Thank you. Would you mind adding the full link to the DEPR in your commit comment? Providing the ticket link in your commit description (not the title) is probably a good pattern to follow as you approach this work. Thanks again. |
* feat: ignore v2 xblocks in independance contract * docs: add documentation as to failure point
…penedx#32590) Otherwise we're not really respecting the package-lock file and won't get repeatable results. Also: - Clean up old error handling for npm<3. Were on npm 8 now. Probably can get rid of this. - Use the shorthand `npm ci` rather than `npm clean-install` just for consistency with code elsewhere. - Update comments in tests to be explicit about use of ci rather than install
…ional) (openedx#32552) Plus remove a few unused and indirect dependencies
* feat: add pagination in course enrollment list API * refactor: enrollment course list API * refactor: follow best practice in course enrollment list API
Use an existing 3rd party library to generate RST docs from the existing swagger.yaml file.
* This project was not being published anywhere. * The previous commit that adds openapi generation capabilities to the `docs/guides` replaces what this was doing.
* Swagger was renamed to OpenAPI at some point so use the new name for clarity. * Prefix with `lms` to make it clear that these are the APIs from the LMS and may not all be available in the CMS.
Since they were not used anywhere, it is ideal to delete them. It would be redundant to keep them.
Don't assume that there will be an extra `context` kwarg when using the bookmark serializer. We use it this way in the current code but that's specific to us and not comon to all serializers. There are a lot of API documentation tools that automate introspecting serializers but they won't know that they have to send in a `context` to the serializer. To make this serializer behave more like other serializers without changing the behavior, we just need to check that the `context` value is defined before we dig into it. In the case that there is no `context` we just treat it the same as if there is no `request` in the `context`.
security patch Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: awais786 <[email protected]>
openedx#32645) * feat: added endpoint for context needed for recommendations experiment * chore: Removed unnecessary decorator
Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`
* fix: mathjax resize on sindow resize * fix: add resize for mathjax in wiki and cms
This PR addresses the renaming of the contentstore/xblock_services folder to contentstore/xblock_storage_handlers as a follow-up to PR openedx#32282. The renaming is done to prevent naming conflicts with xblock runtime services and to make the purpose of the files more understandable. The file xblock_service.py has been renamed to view_handlers.py to better reflect its functionality. Justification and Future Refactoring Outlook: The xblock_storage_handlers folder contains service methods that implement the business logic for view endpoints located in contentstore/views/block.py. It is renamed to xblock_storage_handlers to reflect its responsibility of handling storage-related operations of xblocks, such as creation, retrieval, and deletion. The view_handlers.py file includes business methods called by the view endpoints. These methods, such as handle_xblock, delete_orphans, etc., interact with the required modulestore methods, handle any errors, and aggregate and serialize data for the response. The term 'handler' in the context of 'view_handlers.py' represents methods that facilitate business logic for view endpoints. It is critical to note the distinction between these 'handler methods' and the xblock_handler method. The xblock_handler is a view endpoint itself, residing in contentstore/views/block.py, and is well known in this context. Although its name might suggest otherwise, it is not a handler in the sense of the 'handler methods' we've defined in 'view_handlers.py'. To maintain consistency with existing naming conventions, it remains as xblock_handler.
Co-authored-by: Jesper Hodge <[email protected]>
I think there is some mistake I make another DEPR request |
@Yagnesh1998 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
Description:
The setting JWT_AUTH_REFRESH_COOKIE is meaningless and unused and should be cleaned up to avoid confusion.
In the very early days of introducing MFEs, we thought we were going to need this cookie in addition to the JWT cookie. However, it turned out we didn't need it, but the setting stuck around the contagion of it (being in cookiecutter and other template libraries) has resulted in it uselessly being copied to many repos.
Supporting information:
as per the original ticket openedx/public-engineering#190, this setting is removed.