Social platform URL update #33500
Social platform URL update #33500
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @Inferato! 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. |
mphilbrick211
added
the
needs test run
mphilbrick211
removed
the
needs test run
mphilbrick211
added
the
needs test run
|
Hi @Inferato! It looks like there's a failing check - would you mind taking a look? |
mphilbrick211
removed
the
needs test run
| @@ -4190,8 +4190,8 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring | |||
| }, | |||
| 'twitter': { | |||
| 'display_name': 'Twitter', | |||
There was a problem hiding this comment.
It seems like this value should also be renamed to "X", doesn't it?
There was a problem hiding this comment.
@dyudyunov @Inferato I think so, yep.. and there's a couple more references that could be fixed here too:
- 1 mention in cms.envs.common
- 1 enum label in certificates.models
- 2 mentions in certificates/_accomplishment-banner.html
- student/tests/test_views.py and whatever templates are driving this test.
And maybe more? Mentions made in js/fixtures or js/spec can be ignored I think (e.g. 1, 2), since they're only used in testing.
| @@ -4190,8 +4190,8 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring | |||
| }, | |||
| 'twitter': { | |||
| 'display_name': 'Twitter', | |||
There was a problem hiding this comment.
@dyudyunov @Inferato I think so, yep.. and there's a couple more references that could be fixed here too:
- 1 mention in cms.envs.common
- 1 enum label in certificates.models
- 2 mentions in certificates/_accomplishment-banner.html
- student/tests/test_views.py and whatever templates are driving this test.
And maybe more? Mentions made in js/fixtures or js/spec can be ignored I think (e.g. 1, 2), since they're only used in testing.
|
Sorry for the delay. Yes, please and thank you :) |
|
I checked this PR again and found that:
My first thought was that we could go with current changes and ignore the rest of Twitter mentions in the platform for now because we're updating only the Profile/Account social links validation. But maybe we should allow using both x.com and twitter.com URLs here. Or even decline this PR at all. Please share your thoughts! |
|
@Inferato Ugh.. this is a tricky call.
Ya, I can't reproduce that anymore either.. And so I expect most people will be trying to use twitter.com links as social links, at least for now. So I think we should close this issue, and re-open it if Twitter ever gets its act together :) Ok with you? |
I agree ☝️ |
|
@mphilbrick211 @pomegranited Could you close the PR (considering previous comments)? The author is not available anymore |
|
@Inferato 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
Twitter url changed to 'x.com'
Related: open-release/quince.master