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

feat: adds TenantAwareLinkRenderStarted filter #32014

Closed

Conversation

JuanDavidBuitrago
Copy link
Contributor

@JuanDavidBuitrago JuanDavidBuitrago commented Mar 31, 2023

Description

This PR introduces a new openedx-filters event org.openedx.learning.tenant_aware_link.render.started.v1. This is implemented to guaranty multi tenancy in eox-tenant in LMS_ROOT_URL from Studio to LMS, but eox-tenant is only an example would consider use.

Impact by the change on:

  • cms/djangoapps/contentstore/views/assets.py
  • common/djangoapps/util/course.py

Supporting information

There are not openedx related tickets. The related PR are:

Testing instructions

The change can be tested with testing instructions in eox-tenant plugin.

Deadline

"None"

Other information

⚠️

Warning: Once accepted this PR, it has to be merged only after openedx/openedx-filters#77 is merged and the requirements in this PR are updated.

⚠️

Settings

EDXAPP_EXTRA_REQUIREMENTS:
  - name: git+https://github.com/eduNEXT/eox-tenant.git@JDB/add_tenant_aware_filter

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 31, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Mar 31, 2023

Thanks for the pull request, @JuanDavidBuitrago! 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:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as ready for review April 3, 2023 13:19
@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as draft April 12, 2023 14:44
@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/add_tenant_aware_filter branch 7 times, most recently from 0f5f3bb to 3bd613b Compare April 18, 2023 10:58
@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as ready for review April 19, 2023 07:45
@mphilbrick211
Copy link

Hi @JuanDavidBuitrago! Just checking in to see if you plan to look into the failing tests? Once all checks are green, we can get this reviewed for you. Thanks!

@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/add_tenant_aware_filter branch 8 times, most recently from 77933a5 to e74f948 Compare April 28, 2023 09:39
@JuanDavidBuitrago
Copy link
Contributor Author

Hi @JuanDavidBuitrago! Just checking in to see if you plan to look into the failing tests? Once all checks are green, we can get this reviewed for you. Thanks!

Hi @mphilbrick211!

I had to make a temporal change in requirement to pass the test, but ⚠️ these have to change after this PR (openedx/openedx-filters#77) is merged.

Thanks for your time!

@JuanDavidBuitrago
Copy link
Contributor Author

Hi @mariajgrimaldi and @felipemontoya,

Could you review this?

@mphilbrick211 mphilbrick211 added the blocked by other work PR cannot be finished until other work is complete label Jun 21, 2023
@mphilbrick211
Copy link

Hi @JuanDavidBuitrago! Just following up on this :)

@mariajgrimaldi
Copy link
Member

@mphilbrick211: I spoke internally with @JuanDavidBuitrago's team, and they told me they didn't have the capacity to take this on right now. I'll move this PR to draft to give them time to work on it. Thanks!

@mariajgrimaldi mariajgrimaldi marked this pull request as draft September 7, 2023 14:44
@JuanDavidBuitrago
Copy link
Contributor Author

Hi @mphilbrick211 @mariajgrimaldi, thank you for your patience.

@mphilbrick211
Copy link

Closing this for now. We can reopen in the future if needed. Thanks!

@openedx-webhooks
Copy link

@JuanDavidBuitrago 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.

@mphilbrick211 mphilbrick211 added closed inactivity PR was closed because the author abandoned it and removed blocked by other work PR cannot be finished until other work is complete labels Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed inactivity PR was closed because the author abandoned it open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants