Skip to content

Conversation

@suhaibabsi-inst
Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst commented Dec 16, 2025

refs: MBL-19564
affects: Student, Teacher, Parent
builds: Student, Teacher, Parent
release note: Enhanced searching in "Find your school" screen.

Approach

Accounts searching enhanced in the main two ways:

  • Increasing the page size for the request of accounts search. From 50 to 100 to match what we have on Android.
  • Sorting search results to relocate those with names starting with search query value to be top of the list.

Not this logic is confirmed by support engineers 👍

Test Plan

See ticket's description and use test account mentioned there (it has 50+ account lookup defined).

Checklist

  • Follow-up e2e test ticket created
  • A11y checked
  • Tested on phone
  • Tested on tablet
  • Tested in dark mode
  • Tested in light mode
  • Approve from product

refs: MBL-19564
affects: Student, Teacher, Parent
builds: Student, Teacher, Parent
release note: Enhanced searching in "Find your school" screen.
@suhaibabsi-inst suhaibabsi-inst self-assigned this Dec 16, 2025
@claude
Copy link

claude bot commented Dec 16, 2025

Claude Code Review

Updated: 2025-12-16

Review Result

✅ Approved.

Summary:

  • Per-page increased from 50 to 100 to match Android behavior
  • Query-based sorting added to promote results matching search term prefix
  • Tests updated correctly to verify new behavior
  • Code is correct and test suite passes

@inst-danger
Copy link
Contributor

inst-danger commented Dec 16, 2025

Warnings
⚠️ One or more files are below the minimum test coverage 50%

Release Note:

Enhanced searching in "Find your school" screen.

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

MBL-19564

Coverage New % Master % Delta
Canvas iOS 91.1% 81.06% 10.04%
Horizon/Horizon/Sources/Common/View/CourseDetailsAccessibility.swift 0% -- --

Generated by 🚫 dangerJS against bfe25a4

@inst-danger
Copy link
Contributor

inst-danger commented Dec 16, 2025

Builds

Commit: Merge branch 'master' into bugfix/MBL-19564-Domain-Lookup-Search-Enhanced (bfe25a4)
Build Number: 1056
Built At: Dec 25 11:20 CET (12/25 03:20 AM MST)

Student
Teacher
Parent

Copy link
Contributor

@petkybenedek petkybenedek 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

@suhaibabsi-inst suhaibabsi-inst marked this pull request as ready for review December 25, 2025 09:46
@claude
Copy link

claude bot commented Dec 25, 2025

Claude Code Review

✅ No critical issues found

  • Page size increase (50 → 100) aligns with Android implementation
  • Query-prefix sorting implementation is correct and preserves stable ordering
  • Case-insensitive matching with trimmed() extension properly used
  • Comprehensive test coverage validates sorting logic with edge cases (empty query, no matches, uppercase)
  • Backward-compatible change with no breaking implications

🟢 Approved

Copy link
Collaborator

@vargaat vargaat 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 on functionality, I had a small performance related remark.


return enumerated()
.sorted { result1, result2 in
let qValue = query.lowercased().trimmed()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this to be before the return enumerated() line. The value here is constant across the sorting and we calculate the value on the main thread each time the closure is called.

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