Fix support link empty bug #26140
Fix support link empty bug #26140
Conversation
If support link is not sent, configuration helper returns empty dictionary instead of string. This causes issue on frontend when it tries to set href using this link
| @@ -180,7 +180,7 @@ def _log_and_raise_inactive_user_auth_error(unauthenticated_user): | |||
| error_code='inactive-user', | |||
| context={ | |||
| 'platformName': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), | |||
| 'supportLink': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK) | |||
| 'supportLink': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK) or '' | |||
There was a problem hiding this comment.
Shouldn’t it fallback to settings.SUPPORT_SITE_LINK if it's not set in the site configuration which actually is an empty string?
There was a problem hiding this comment.
currently on my local devstack both these values are not set
- My site config do not have anything
- SUPPORT_SITE_LINK = ''
but when I debug and run this statement as whole it returns {}
> configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK)
{}There was a problem hiding this comment.
That is I guess a bug in this code https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/site_configuration/helpers.py#L118-L129 which is expecting to raise an exception upon updating a non-dict object.
There was a problem hiding this comment.
Will look into it and update the change
There was a problem hiding this comment.
Thank you!
One thing to notice here is that this only happens if the default value is an empty string instead of None. If the configuration is not defined and the default is a None then it works as expected.
|
Your PR has finished running tests. There were no failures. |
|
📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build. We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master. This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself: If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal). |
|
Fixed the issue in another PR: https://github.com/edx/edx-platform/pull/28577 |
If support link is not sent, configuration helper returns empty dictionary instead of string. This causes issue on frontend when it tries to set href using this link.