Skip to content

Conversation

@tamaskozmer
Copy link
Contributor

@tamaskozmer tamaskozmer commented Dec 4, 2025

Test plan:
In the ticket

refs: MBL-19568
affects: Student
release note: Fixed folder and quiz bookmarking.

Checklist

  • Tested in dark mode
  • Tested in light mode

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

- Disable bookmarking for non-root folders to prevent routing issues with embedded links
- Add route for new quizzes opened from quiz list to Assignment Details

Test Plan:
- Verify bookmarking is only available in root folder
- Verify new quizzes route to Assignment Details correctly

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link

@claude claude bot left a 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 fixes the bookmarking behavior for file lists and adds routing support for new quizzes from the quiz list. The changes are focused and address specific navigation issues.

Issues Found

  • Route pattern syntax error in RouteMatcher.kt:408 - Missing leading / in route pattern which may prevent URLs from matching correctly

Positive Feedback

FileListFragment.kt:

  • Good fix for the bookmark behavior - adding the folder?.isRoot == true check ensures that only root folder views in courses/groups are bookmarkable, which is the appropriate UX behavior
  • The null-safe navigation operator usage (folder?.isRoot) is correct and handles the case where folder might be null

RouteMatcher.kt:

  • The comment explaining the purpose of the new route is helpful for future maintainers
  • The route correctly maps QuizListFragment → AssignmentDetailsFragment for new quizzes, which aligns with the architecture where new quizzes are displayed on assignment details

Code Quality

  • Changes follow existing patterns in the codebase
  • Code is concise and focused on the specific issues being addressed
  • Null safety is properly handled with the safe call operator

Testing Recommendations

While no test files were modified, consider:

  • Manual testing of bookmarking file list views in courses/groups at root and non-root levels
  • Manual testing of navigation from quiz list to new quiz assignment details
  • Verifying the route pattern matches expected URLs once the syntax issue is fixed

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

📊 Code Coverage Report

⚠️ Student

  • PR Coverage: 42.76%
  • Master Coverage: 42.77%
  • Delta: -0.01%

✅ Teacher

  • PR Coverage: 25.45%
  • Master Coverage: 25.45%
  • Delta: +0.00%

⚠️ Pandautils

  • PR Coverage: 22.56%
  • Master Coverage: 22.56%
  • Delta: -0.00%

📈 Overall Average

  • PR Coverage: 30.26%
  • Master Coverage: 30.26%
  • Delta: -0.00%

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

🧪 Unit Test Results

✅ 📱 Student App

  • Tests: 1226 total, 0 failed, 0 skipped
  • Duration: 0.000s
  • Success Rate: 100%

✅ 🌅 Horizon

  • Tests: 449 total, 0 failed, 0 skipped
  • Duration: 33.538s
  • Success Rate: 100%

✅ 📦 Submodules

  • Tests: 2418 total, 0 failed, 0 skipped
  • Duration: 45.658s
  • Success Rate: 100%

📊 Summary

  • Total Tests: 4093
  • Failed: 0
  • Skipped: 0
  • Status: ✅ All tests passed!

Last updated: Thu, 04 Dec 2025 15:01:20 GMT

Copy link
Contributor

@kdeakinstructure kdeakinstructure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA 👍

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

Student Install Page

Copy link
Contributor

@adamNagy56 adamNagy56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA +1

@tamaskozmer tamaskozmer merged commit 9dc350f into master Dec 9, 2025
91 of 97 checks passed
@tamaskozmer tamaskozmer deleted the MBL-19568-fix-bookmark-issues branch December 9, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants