fix: prioritize current site setting for org value

[navinkarkera]

Description

If multiple tenant configs are enabled for a given org, fetching value from site config for the organization should return current site config value if the org is included in the org_filter.

Private-ref: BB-7927

Testing instructions

See open-craft#3

Checklist for Merge

• Tested in a remote environment
• Updated documentation
• Rebased master/main
• Squashed commits

[kaustavb12]

👍

• I tested this: Verified that the changes here are same as in the internal PR refactor: prioritize current site setting for org value open-craft/eox-tenant#3
• I read through the code
• I checked for accessibility issues
• Includes documentation
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

[kaustavb12]

@navinkarkera Could you please add our internal ticket number here too as private ref, for easy reference.

[bra-i-am]

Hello @navinkarkera and @kaustavb12, I hope you are doing well. We thank you for contributing to this project and apologize for the delay in reviewing it due to some urgent matters.

The solution seems logical. However, before moving forward, I would like to test it to gain a more comprehensive understanding of the expected behavior and the match with all the use cases.

I followed the steps in the referenced PR open-craft#3, but it didn't work for me. In order to be more specific, here are the steps I followed:

1. Ran make {lms,cms}-up
2. Cloned your repo fork (to be allowed to use the branch navin/upstream/current-site-priority-for-org-value), went into the shells in the LMS & CMS, and ran the pip install -e command to the repo
3. Ran the two migrations despite only the LMS executed the changes (expected behavior)
4. Created the two Routes with their corresponding tenant configs the same as you did
5. Updated the redirect URLs in thestudio-sso application
6. Added the setting in the private.py files
7. Reseted

This is what I receive when I try to see the link when View live is hovered:

These are my tenant configs with the settings you told in open-craft#3 and there are shown the domains I use too

To test this PR I mounted a new environment in devstack, please let me know if maybe you have any other setting/configuration I'm missing or maybe you have an idea about what can I do to run this PR properly.

Perhaps, if you have tested this PR using Tutor, could you please guide us a little through the process you followed?

I will be closely monitoring your response to continue with my review or provide any more information if needed.

Thank you so much ✨

[navinkarkera]

Hello @bra-i-am, I just rebased the branch with master and followed the steps mentioned in open-craft#3 to test in master devstack. It worked for me and I did not do anything special.

Please make sure that you are using open-craft:navin/upstream/current-site-priority-for-org-value branch

[bra-i-am]

@navinkarkera, thank you so much for your answer. I could not reproduce the behavior before because of some config mistakes I made with my routes, but I finally got to solve them.

The change introduced in this PR makes all the sense to me and according to some tests I ran, there are no conflicts with other test cases. I'll announce this to my teammates to continue with the merge.

Thanks a lot for your contribution ✨

Evidence

=====================================

FYI

• I ran the PR using Tutor in dev mode changing the variable CMS_HOST same as LMS_HOST in the config.yml file (e.g. CMS_HOST: local.overhang.io)
• I created the routes tenant-a.local.overhang.io and tenant-b.local.overhang.io
• I created the Tenant Configs according to the shown here but I changed the URL to the one of the route that is being configured. For example the lms_configs for the Tenant A:

{
  "EDNX_USE_SIGNAL": true,
  "LMS_BASE": "tenant-a.local.overhang.io:8000",
  "LMS_ROOT_URL": "http://tenant-a.local.overhang.io:8000",
  "SOCIAL_AUTH_EDX_OAUTH2_PUBLIC_URL_ROOT": "http://tenant-a.local.overhang.io:8000",
  "course_org_filter": [
    "edX"
  ]
}

[MaferMazu]

Looks good to me. I will change the commit to fix because we are changing the previous behavior.

[MaferMazu]

@navinkarkera, can you sign your commit, please?

Ref: https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

[navinkarkera]

@MaferMazu Done.