Development: Fix LTI content selection table not loading
#9965
Conversation
github-actions
bot
added
the
client
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
raffifasaro
added
lock:artemis-test3
deploy:artemis-test3
and removed
lock:artemis-test3
labels
|
github-actions
bot
added
the
deployment-error
raffifasaro
removed
the
deployment-error
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
raffifasaro
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/lti/lti13-exercise-launch.component.ts (1)
151-156: Consider adding unit tests for URL handling.The new URL handling logic introduces different paths that should be covered by unit tests.
Would you like me to help generate comprehensive unit tests for the
replaceWindowLocationWrappermethod? The tests should cover:
- Direct path for '/lti/select-course'
- Absolute URLs
- Relative URLs
- Malformed URLs
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/webapp/app/lti/lti13-exercise-launch.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/lti/lti13-exercise-launch.component.ts (1)
🔇 Additional comments (1)
src/main/webapp/app/lti/lti13-exercise-launch.component.ts (1)
151-156: Verify the impact on LTI navigation flows.
The changes modify how URLs are handled in the LTI launch flow. Let's ensure this doesn't affect other LTI navigation scenarios.
✅ Verification successful
Let me gather more context about the LTI launch flow to make a more informed decision.
Let me search for more specific information about the LTI launch flow and URL handling.
The URL handling changes in the LTI launch flow are safe and properly implemented
The changes handle two distinct cases correctly:
- Direct navigation to
/lti/select-coursepreserves the exact path - Other URLs are properly parsed to extract their pathname using the URL API
The implementation aligns with both frontend routing patterns and backend LTI configuration, as evidenced by:
- The select-course path is a well-defined constant
LTI13_DEEPLINK_SELECT_COURSE_PATHin the backend - The URL handling is consistent with existing LTI navigation flows
- No other LTI components are affected by this URL parsing change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other LTI-related navigation code that might be affected
echo "Searching for LTI navigation patterns..."
rg -t ts "router\.navigate.*lti" --no-filename
echo "Searching for URL construction patterns in LTI components..."
fd -e ts -e html . | grep -i "lti" | xargs rg "new URL\(" --no-filename
Length of output: 740
Script:
#!/bin/bash
# Search for the complete implementation of lti13-exercise-launch component
echo "Searching for LTI launch component implementation..."
ast-grep --pattern 'export class Lti13ExerciseLaunchComponent {
$$$
}'
# Search for URL handling in other LTI components
echo "Searching for URL handling in LTI components..."
rg -t ts "url.*=.*" --no-filename -A 2 | grep -i "lti"
Length of output: 1046
Script:
#!/bin/bash
# Search for the specific file content to understand the context better
echo "Content of lti13-exercise-launch.component.ts:"
rg -A 10 -B 10 "if \(url === '/lti/select-course'\)"
# Search for LTI-related URL handling patterns
echo "Searching for LTI URL patterns..."
rg -t ts "lti.*url|url.*lti" -A 2
# Search for router navigation related to select-course
echo "Searching for select-course navigation patterns..."
rg "/lti/select-course" -A 2
Length of output: 5164
krusche
changed the title
Development: Fix lti content selection table not loadingDevelopment: Fix LTI content selection table not loading
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Content selection table for creating lti learning activities was not loading correctly
Description
Change the launch method to take the url path and not a full URL in case of table because of different parameter format given by the table.
Steps for Testing
Prerequisites:
Make sure you are logged in to Artemis with a Test User 16)
Navigate to Moodle and login with the same test user (Test User 16}, same PW as for the Artemis Test Server)
Go to My Courses and open the lti test course
Activate Edit mode in top right corner
Add a learning activity somewhere by clicking on the blue +
Select TS1
Click on "Select content"
A modal should appear with either a course or a message that there are no lti courses for your user.
(As long as the table does not display a "Am Starten..." or "Starting" message, everything works as expected)
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
'/lti/select-course'.Bug Fixes