[MBL-18950][All] Add E2E Tests for Inbox Signature FAB Functionality

[adamNagy56]

Summary

Added comprehensive E2E test coverage for Inbox Signature feature accessed via FAB buttons.

The test validates:

• User 1 sets signature → appears in all FAB compose locations
• Switch to User 2 → no signature shown (user isolation)
• User 2 sets different signature → appears correctly
• Verifies MBL-18929 fix: signatures work with FAB buttons

refs: MBL-18950
affects: All
release note:

Checklist

• Run E2E test suite

[claude]

Pull Request Review: MBL-18950

Summary

This PR adds comprehensive E2E test coverage for the Inbox Signature feature when accessed via FAB (Floating Action Button) buttons across all three apps (Parent, Student, Teacher). The tests validate that signatures are properly displayed and isolated per user, verifying the fix for bug MBL-18929.

Code Quality & Best Practices ✅

Strengths:

• Consistent patterns: The tests follow existing E2E test patterns in the codebase perfectly, using the same structure, logging tags (PREPARATION_TAG, STEP_TAG, ASSERTION_TAG), and page object interactions
• Well-structured: Clear test flow with proper setup, execution, and verification phases
• Descriptive logging: Excellent use of Log.d statements that document each step, making test failures easy to debug
• Proper annotations: Correct use of @E2E, @TestMetaData with appropriate priority (BUG_CASE), feature categories, and references to bug ticket MBL-18929
• Follows repository conventions: Adheres to the patterns described in CLAUDE.md for test organization and structure

Code style observations:

• The code is clean, readable, and follows Kotlin best practices
• Variable naming is clear and descriptive
• Test method names follow the convention test{Feature}{Scenario}E2E

Test Coverage ✅

Excellent coverage:

• Multi-user isolation: Tests verify signatures are user-specific, not shared between users
• Multiple FAB locations: Each app tests signature display from various FAB entry points:
  • Parent app: Grades tab, Syllabus tab, Summary tab, Assignment Details
  • Student app: People Details page (Compose button)
  • Teacher app: Student Context page (Compose button)
• Full workflow: Tests both signature creation and verification that it persists correctly
• Negative testing: Verifies that User 2 does NOT see User 1's signature (user isolation)

Potential Issues & Suggestions

1. Missing newline at end of files ⚠️

All three test files are missing a newline at the end (line 684 in parent, 887 in student, 827 in teacher). This is a common best practice and may cause issues with some Git tools.

Recommendation: Add a newline at the end of each file.

2. Minor typo in student test (line 723) 🔍

val data = seedData(students = 2, teachers = 1 ,courses = 1)

There's an extra space before the comma: 1 ,courses should be 1, courses

Recommendation: Fix spacing for consistency.

3. Test execution time consideration ⏱️

These are very comprehensive E2E tests that:

• Seed multiple users and courses
• Perform full login/logout flows twice
• Navigate through multiple screens repeatedly
• Test 4+ different FAB locations per app

Observation: While thorough, these tests may take considerable time to execute. This is expected for E2E tests, but worth noting for CI pipeline considerations.

4. Incomplete checklist in PR description 📋

The PR description shows - [ ] Run E2E test suite as unchecked.

Recommendation: Ensure E2E tests have been run successfully before merging.

5. Parent app specific addition 🔍

Parent app test includes this line:

protected val inboxComposePage = InboxComposePage(composeTestRule)

in ParentComposeTest.kt:69

Good catch: This page object was missing and needed to be added. Verify that this doesn't conflict with any other page object or cause test issues in other parent tests.

Performance Considerations ✅

• No performance concerns with the test code itself
• Tests use appropriate waits (waitForRender(), retryWithIncreasingDelay() where needed in other tests)
• Data seeding is efficient with minimal setup data
• Navigation patterns are straightforward without unnecessary delays

Security Concerns ✅

• No security issues identified
• Tests properly use CanvasNetworkAdapter.canvasDomain for domain configuration
• Authentication flows follow existing patterns
• No hardcoded credentials or sensitive data
• User isolation is properly tested, which is a security-positive feature

Bug Verification ✅

The tests properly validate the fix for MBL-18929:

• ✅ Signatures appear when composing from FAB buttons
• ✅ Signatures are user-specific and properly isolated
• ✅ Tests cover all major FAB entry points across three apps

Final Recommendations

Required before merge:

1. Add newline at end of all three test files
2. Fix spacing in student test seedData() call (line 723)
3. Verify E2E tests pass successfully (update PR checklist)

Nice to have:

4. Consider adding a comment explaining why different FAB locations are tested (validates MBL-18929 fix that signatures work across all FAB entry points)
5. Document expected test execution time in commit message or test comments

Verdict: ✅ Approve with minor fixes

This is a well-crafted PR that adds valuable test coverage for an important bug fix. The tests are thorough, follow existing patterns perfectly, and will prevent regression of the signature FAB functionality. The issues identified are minor and easily addressable.

Great work on the comprehensive test coverage! 🎉

---

Review generated by Claude Code

[inst-danger]

Parent Install Page

[github-actions]

📊 Code Coverage Report

✅ Student

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

✅ Teacher

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

✅ Pandautils

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

---

📈 Overall Average

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

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[kdeakinstructure]

LGTM!