-
Notifications
You must be signed in to change notification settings - Fork 121
[MBL-19760][S/T/P] Update Community links #3872
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
Conversation
refs: MBL-19760 builds: Student, Teacher, Parent affects: Student, Teacher, Parent release note: none
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
This PR updates Canvas Community help links from the deprecated community.canvaslms.com domain to the new community.instructure.com domain. The changes are straightforward and improve the user experience by directing users to the correct, up-to-date help resources.
Positive Aspects
- Clean, focused changes that update URLs consistently across three key areas (login help, Canvas guides search, and wrong app screen)
- The new URLs are valid and point to appropriate help resources
- Changes follow the existing code patterns and conventions
Issues Found
- Incomplete URL migration: Several files still reference the old
community.canvaslms.comdomain and should be updated for consistency:Core/Core/Features/Courses/SmartSearch/View/CourseSmartSearchHelpView.swift(lines 72, 79, 84)Core/Core/Common/CommonUI/CoreWebView/Model/CoreWebViewJSInjections.swift(line 141 comment)Core/CoreTests/Features/Profile/Help/View/HelpViewTests.swift(line 29)Core/CoreTests/Features/Login/LoginWebViewControllerTests.swift(line 74)
Recommendations
- Consider updating all remaining references to
community.canvaslms.comin the codebase to ensure consistency - Verify that the new URLs are accessible and return the expected content
- Consider adding a test to verify the help URLs are valid and reachable
Security & Performance
No security concerns or performance impacts identified.
BuildsCommit: Localization files (b7adb5a) |
rh12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something, the updated localization files should be included in the PR as well
|
Pushed the loc files |
thank you, @petkybenedek! |
vargaat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA+1
refs: MBL-19760
builds: Student, Teacher, Parent
affects: Student, Teacher, Parent
release note: none
Updated all community links
Test plan