[MBL-19335][Student] - Discussion Checkpoint E2E test for Assignment List/Details

[kdeakinstructure]

Discussion checkpoint assignment list ande details related E2E test.
Add some testtags + wrap DueDateBlock into a Column to be able to identify it.
Added graphql mutation queries (it will be used once the seeding will be fixed - INTEROP-9901 ticket)

refs: MBL-19335
affects: Student
release note:

• Run E2E test suite

[inst-danger]

Parent Install Page

[github-actions]

📊 Code Coverage Report

✅ Student

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

✅ Teacher

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

✅ Pandautils

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

---

📈 Overall Average

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

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

PR Review: Discussion Checkpoint E2E Test

Summary

This PR adds E2E tests for discussion checkpoints in the assignment list and details pages. The implementation includes GraphQL mutation support for creating discussions with checkpoints and necessary UI test tag additions.

---

Code Quality & Best Practices

✅ Strengths

1. Good test structure - The new E2E test follows existing patterns in DiscussionsE2ETest.kt
2. Proper use of page objects - Leverages the page object pattern consistently
3. Clear logging - Uses STEP_TAG, ASSERTION_TAG, and PREPARATION_TAG for test clarity
4. Test metadata - Properly annotated with @TestMetaData including the new SecondaryFeatureCategory.DISCUSSION_CHECKPOINTS
5. Compose test assertions - Good use of Compose test selectors with proper test tags

⚠️ Issues & Suggestions

1. Hardcoded Test Data (apps/student/src/androidTest/java/com/instructure/student/ui/e2e/classic/DiscussionsE2ETest.kt:176-180)

val courseId = 3594441L
val courseName = "Kristof Deak Dedicated Test Course"
val discussionWithCheckpointsTitle = "Discussion with Checkpoints"
val discussionWithCheckpointsId = 22475794L

Issue: Hardcoded course IDs and names are brittle and environment-specific. This test will fail on other environments or if this specific course is deleted.

Recommendation:

• Use the seeded data from seedData() to create/use a course
• Create the discussion dynamically once GraphQL mutation is working (see line 185 comment)
• Remove unused discussionWithCheckpointsId variable

Suggested fix:

val data = seedData(students = 1, teachers = 1, courses = 1)
val student = data.studentsList[0]
val teacher = data.teachersList[0]
val course = data.coursesList[0]
val discussionWithCheckpointsTitle = "Discussion with Checkpoints"

// Once INTEROP-9901 is resolved, use this instead:
// val discussion = DiscussionTopicsApi.createDiscussionTopicWithCheckpoints(
//     course.id, teacher.token, discussionWithCheckpointsTitle, "Test Assignment with Checkpoints"
// )

---

2. Typo in Filename (libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/ChekcpointItem.kt)

Issue: File is named ChekcpointItem.kt instead of CheckpointItem.kt (missing 'h')

Recommendation: Rename the file to fix the typo:

git mv libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/ChekcpointItem.kt \
       libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/CheckpointItem.kt

---

3. Missing Error Handling (automation/dataseedingapi/src/main/kotlin/com/instructure/dataseeding/api/DiscussionTopicsApi.kt:113-141)

Issue: The createDiscussionTopicWithCheckpoints function has extensive error logging but doesn't provide a useful return value. It returns Unit but throws exceptions, making it hard to use in tests.

Recommendation:

• Return the created discussion topic data so tests can use the actual IDs
• Consider a more structured return type

Suggested improvement:

fun createDiscussionTopicWithCheckpoints(
    courseId: Long, 
    token: String, 
    discussionTitle: String, 
    assignmentName: String
): DiscussionTopicGraphQLResult {
    // ... existing code ...
    
    val discussionTopic = response.data?.createDiscussionTopic?.discussionTopic
        ?: throw Exception("Discussion topic not created")
    
    return DiscussionTopicGraphQLResult(
        id = discussionTopic._id.toLongOrNull() ?: 0L,
        title = discussionTopic.title ?: discussionTitle
    )
}

data class DiscussionTopicGraphQLResult(val id: Long, val title: String)

---

4. Test Dependency on External State (apps/student/src/androidTest/java/com/instructure/student/ui/e2e/classic/DiscussionsE2ETest.kt:213)

Issue: Test relies on manually enrolling a student to a pre-existing course rather than using seeded data.

Recommendation: This creates a dependency on external test data and makes the test non-isolated. Once the GraphQL mutation is working, the test should be fully self-contained using seeded data.

---

5. Inconsistent Test Tag Naming (libs/pandautils/src/main/java/com/instructure/pandautils/features/assignments/details/composables/DueDateReminderLayout.kt:64-69)

Issue: Test tag uses string interpolation with dueLabel which could contain spaces or special characters:

.testTag("dueDateColumn-${reminderViewState.dueLabel}")
.testTag("dueDateHeaderText-${reminderViewState.dueLabel}")

Recommendation: This is actually fine since it's being used consistently in the test file. Just ensure the matching assertion uses the exact same format (which it does in StudentAssignmentDetailsPage.kt:71).

---

Potential Bugs

🐛 Unused Import (apps/student/src/androidTest/java/com/instructure/student/ui/e2e/classic/DiscussionsE2ETest.kt:26)

The DiscussionTopicsApi is imported but the actual GraphQL creation is commented out. While this is intentional (waiting on INTEROP-9901), it should be cleaned up once the feature is available.

---

🐛 Schema File Changes (automation/dataseedingapi/src/main/graphql/com/instructure/dataseeding/schema.json)

Issue: The schema.json file shows 49,128 additions and 30,197 deletions, which is a massive change.

Recommendation:

• Verify this is the correct updated schema from Canvas
• Consider if this needs to be in a separate PR since it's such a large change
• Ensure the schema update didn't break any existing GraphQL queries in the codebase

---

Performance Considerations

✅ Good Practices

1. Proper use of waitForIdle() - The test waits appropriately after actions
2. Efficient selectors - Compose test selectors use useUnmergedTree = true where needed for performance
3. Minimal network calls - Test structure minimizes unnecessary API calls

⚠️ Potential Issues

1. Test execution time - The test enrolls a student to a course synchronously, which could be slow. Consider if this can be done in test setup.

---

Security Concerns

✅ No Major Issues

• No sensitive data exposure
• Test tokens are properly scoped
• No SQL injection or XSS vectors

---

Test Coverage

✅ Good Coverage

1. Assignment List - Tests checkpoint display, expand/collapse, and detail views
2. Assignment Details - Tests checkpoint details on the details page
3. UI Components - Good coverage of the CheckpointItem composable with test tags

⚠️ Missing Coverage

1. Edge Cases:
   • What happens if checkpoints have actual due dates instead of "No due date"?
   • What about checkpoints with grades vs. no grades?
   • Error states when GraphQL mutation fails?
2. Unit Tests: The new CheckpointItem composable and updated DueDateReminderLayout should have corresponding unit tests in:
   • libs/pandautils/src/androidTest/java/com/instructure/pandautils/compose/features/assignments/list/AssignmentListScreenTest.kt

---

Recommendations Summary

🔴 Critical (Should fix before merge)

1. Remove hardcoded course IDs and use seeded data
2. Fix filename typo: ChekcpointItem.kt → CheckpointItem.kt

🟡 Important (Should address soon)

1. Add return value to createDiscussionTopicWithCheckpoints() for testability
2. Verify schema.json update is correct and necessary
3. Add unit tests for new UI components

🟢 Nice to have

1. Add edge case tests for different checkpoint states
2. Clean up unused imports when GraphQL feature is ready
3. Consider extracting hardcoded strings to resources

---

Verdict

Overall: Good implementation following project conventions! The main concerns are around test brittleness due to hardcoded data and the typo in the filename. Once these are addressed and the GraphQL mutation is working (per INTEROP-9901), this will be a solid E2E test.

Great work on the comprehensive test logging and proper use of test tags for accessibility! 🎉

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

Code Review: Discussion Checkpoint E2E Test for Assignment List/Details

Overall Assessment

This PR adds E2E test coverage for discussion checkpoints in assignment lists and details pages, along with supporting GraphQL mutations and UI enhancements. The implementation is solid with good test coverage, though there are several areas for improvement.

---

🔍 Code Quality & Best Practices

✅ Strengths

• Good test structure: The new E2E test follows existing patterns with clear PREPARATION, STEP, and ASSERTION tags
• Proper use of Compose test tags: Added test tags (checkpointName, checkpointDueDate, etc.) for reliable UI testing
• Consistent naming conventions: Follows Kotlin and project conventions
• Well-documented: Test steps are clearly logged and easy to follow

⚠️ Areas for Improvement

1. Typo in filename (ChekcpointItem.kt → line 1)
The file is named ChekcpointItem.kt but should be CheckpointItem.kt (missing "p"). This should be renamed for consistency.

2. Hardcoded test data (DiscussionsE2ETest.kt:177-180)

val courseId = 3594441L
val courseName = "Kristof Deak Dedicated Test Course"
val discussionWithCheckpointsTitle = "Discussion with Checkpoints"
val discussionWithCheckpointsId = 22475794L

• These hardcoded values make the test fragile and environment-specific
• The discussionWithCheckpointsId is declared but never used
• Recommendation: Either seed this data programmatically or add clear documentation explaining the test setup requirements

3. Inconsistent test inheritance (DiscussionsE2ETest.kt:40)
The test class was changed from StudentTest() to StudentComposeTest(). While this may be intentional, ensure:

• This change is necessary for Compose-based assertions
• All other tests in the class work with this base class
• Consider if a more descriptive PR description could explain why this change was needed

4. Commented-out code (DiscussionsE2ETest.kt:185-186)

// This will be the GraphQL way of creating a discussion with checkpoints when available. See INTEROP-9901 ticket for details.
//val disc = DiscussionTopicsApi.createDiscussionTopicWithCheckpoints(course.id, teacher.token, "Test Discussion with Checkpoints", "Test Assignment with Checkpoints")

While the comment references a ticket, consider:

• When will this be uncommented?
• Should there be a TODO or FIXME marker?
• The commented line uses course but the test uses hardcoded courseId - these would need to be reconciled

---

🐛 Potential Bugs & Issues

1. Unused GraphQL mutation file (CreateDiscussionTopicWithCheckpoints.graphql)

• The PR adds CreateDiscussionTopicWithCheckpoints.graphql with extensive configuration
• But createDiscussionTopicWithCheckpoints uses CreateDiscussionTopicMinimal instead (DiscussionTopicsApi.kt:105)
• Question: Is CreateDiscussionTopicWithCheckpoints.graphql needed? If yes, when will it be used?

2. GraphQL schema.json changes

• The schema file shows a massive refactor (49128 additions, 30197 deletions)
• The structure changed from {"__schema": {...}} to {"data": {"__schema": {...}}}
• Verify: Was this regenerated correctly? Does this affect other GraphQL operations?

3. Potential NullPointerException (DiscussionTopicsApi.kt:55, 63)

.body()!!

Multiple methods use the !! operator after .body(). If the API call fails or returns null:

• This will throw NPE with unclear error messages
• Recommendation: Add proper error handling or at least more descriptive error messages

4. Test reliability concern (DiscussionsE2ETest.kt:148)

sleep(2000) // Allow some time for entry creation to propagate

• Thread.sleep is already present in the existing test
• Consider if there's a more reliable way to wait (polling, callbacks, etc.)
• This may cause flakiness on slower test environments

5. Missing null safety in date parsing (AssignmentListPage.kt:292-301)
The toDate() extension catches exceptions and returns null, but callers should handle null cases gracefully. The code does handle this, but consider adding explicit null checks for clarity.

---

⚡ Performance Considerations

1. Schema file size

• The schema.json file is extremely large (49K+ additions)
• Question: Is this file committed correctly? Should it be gitignored or compressed?
• Large files in PRs can slow down reviews and increase repo size

2. Efficient compose selectors (AssignmentListPage.kt:146-149)

composeTestRule.onNode(hasTestTag("expandDiscussionCheckpoints") and hasParent(hasAnyDescendant(hasText(discussionTitle))), useUnmergedTree = true)

The selector uses hasAnyDescendant which can be expensive. Consider:

• Can you use a more specific test tag that includes the discussion title?
• This would make lookups faster and more reliable

---

🔒 Security Concerns

No major security issues identified. The changes are primarily test-related and don't introduce security vulnerabilities. However:

1. Sensitive data in logs: The extensive debug logging includes user names, course IDs, etc. Ensure these logs are not exposed in production or CI artifacts
2. Hardcoded credentials: Verify that DATA_SEEDING_ADMIN_TOKEN, DATA_SEEDING_CLIENT_ID, and DATA_SEEDING_CLIENT_SECRET in CanvasNetworkAdapter.kt are properly externalized (they appear to be constants, ensure they're not committed)

---

✅ Test Coverage

Overall: Good

Positive aspects:

• New E2E test covers the complete flow: enrollment → login → navigation → assertions
• Tests both assignment list and details pages
• Validates checkpoint expand/collapse behavior
• Checks grades, due dates, and multiple checkpoint states

Gaps:

1. No unit tests: While E2E tests are valuable, consider adding unit tests for:

   • CheckpointItem composable (screenshot/snapshot tests)
   • assertDiscussionCheckpointDetails logic
   • GraphQL mutation input validation

2. Error cases not tested:

   • What happens if checkpoints have missing data?
   • What if a discussion has only 1 checkpoint instead of 2?
   • Network failures during checkpoint loading?

3. Missing test for the GraphQL creation path: The createDiscussionTopicWithCheckpoints method has extensive error handling but no tests covering it

---

📋 Recommendations Summary

High Priority:

1. Remove or rename the typo in ChekcpointItem.kt
2. Clarify the hardcoded test data approach or seed it programmatically
3. Verify and document the schema.json changes
4. Determine if CreateDiscussionTopicWithCheckpoints.graphql is needed

Medium Priority:
5. Add proper error handling instead of !! operators
6. Consider adding unit tests for new composables
7. Add test coverage for edge cases (1 checkpoint, missing data, etc.)
8. Replace sleep() with more reliable waiting mechanisms

Low Priority:
9. Clean up commented code with TODO markers
10. Optimize compose selectors for better performance
11. Consider documenting the test setup requirements in the test class

---

🎯 Conclusion

This is a solid PR that adds valuable test coverage for discussion checkpoints. The code quality is generally good and follows project conventions. The main concerns are around hardcoded test data, the massive schema.json change, and potential null safety issues. Once these are addressed, this will be a strong addition to the test suite.

Recommendation: Approve with requested changes to address the high-priority items.

Great work on the comprehensive E2E test! 🚀

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

PR Review: Discussion Checkpoint E2E Test for Assignment List/Details

Overview

This PR adds E2E test coverage for Discussion Checkpoints in the Assignment List and Details pages. The changes include GraphQL mutation support for creating discussions with checkpoints, UI test enhancements, and testability improvements to production code.

Code Quality & Best Practices

✅ Strengths

• Good test structure: The new E2E test follows existing patterns with clear logging and step-by-step assertions
• Proper test tagging: Uses @TestMetaData with appropriate categories including the new SecondaryFeatureCategory.DISCUSSION_CHECKPOINTS
• Page object pattern: New methods properly added to page objects (AssignmentListPage, StudentAssignmentDetailsPage, DashboardPage)
• Testability improvements: Added test tags to production code in DueDateReminderLayout.kt and CheckpointItem.kt for better test targeting
• GraphQL implementation: Added Apollo client setup with custom DateTime adapter for proper date handling
• Schema update: Updated GraphQL schema to support discussion checkpoint mutations

⚠️ Issues & Concerns

1. Hardcoded Test Data (Critical)

DiscussionsE2ETest.kt:177-180

val courseId = 3594441L
val courseName = "Kristof Deak Dedicated Test Course"
val discussionWithCheckpointsTitle = "Discussion with Checkpoints"
val discussionWithCheckpointsId = 22475794L

Issue: The test relies on hardcoded course and discussion IDs from a specific test environment. This makes the test:

• Non-portable across environments
• Fragile (fails if data is deleted)
• Not self-contained

Recommendation: Use the seeded data or create the discussion dynamically once the GraphQL mutation is available (as noted in the TODO comment).

2. Unused Variable

DiscussionsE2ETest.kt:180

val discussionWithCheckpointsId = 22475794L

Issue: This variable is declared but never used in the test.

Recommendation: Remove it or use it if needed for future assertions.

3. File Naming Typo

libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/ChekcpointItem.kt

Issue: The filename has a typo: "Chekcpoint" instead of "Checkpoint". While the class name inside is correct (CheckpointItem), the filename doesn't match.

Recommendation: Rename the file to CheckpointItem.kt to match the class name and fix the typo.

4. Thread.sleep in Test

AssignmentsE2ETest.kt:151

Thread.sleep(5000) // Give time for the worker to be enqueued

Issue: Using Thread.sleep is a brittle testing practice that can cause:

• Flaky tests (if 5 seconds isn't enough)
• Slow tests (if less time is actually needed)

Recommendation: Consider using WorkManager testing utilities or IdlingResources to properly wait for asynchronous operations. If this pattern is already established in the codebase, it may be acceptable but should be addressed project-wide.

5. Large Schema File

automation/dataseedingapi/src/main/graphql/com/instructure/dataseeding/schema.json

Impact: Added 19,329 lines and removed 30,197 lines (net: -10,868 lines but still 49,128 total lines). This is a generated file update from copying canvasapi2 schema.

Note: This is expected for schema updates but makes the PR diff very large. Consider if there's a way to automate schema syncing in the future.

6. Commented Code

DiscussionsE2ETest.kt:186

//val disc = DiscussionTopicsApi.createDiscussionTopicWithCheckpoints(course.id, teacher.token, ...)

Issue: While the comment explains why it's disabled (INTEROP-9901), the commented-out code should ideally be removed once the issue is resolved.

Recommendation: Add a TODO comment or ticket reference, or remove entirely since the function signature is visible in DiscussionTopicsApi.kt.

7. Test Assertions Could Be More Specific

StudentAssignmentDetailsPage.kt:69-71

fun assertDiscussionCheckpointDetailsOnDetailsPage(checkpointText: String, dueAt: String) {
    composeTestRule.onNode(hasTestTag("dueDateHeaderText-$checkpointText"), useUnmergedTree = true).assertIsDisplayed()
    composeTestRule.onNode(hasText(dueAt) and hasAnyAncestor(...)).assertIsDisplayed()
}

Suggestion: Consider adding content description assertions or more specific semantic checks to ensure accessibility is properly tested.

Potential Bugs

1. Test Dependency on External Data

The test won't work in fresh environments without the hardcoded course/discussion. This will likely cause CI failures or issues for other developers.

2. Unused GraphQL Function

DiscussionTopicsApi.createDiscussionTopicWithCheckpoints() is implemented but not actually used due to backend limitations (INTEROP-9901). This dead code could confuse future maintainers.

Recommendation: Consider adding a @Deprecated annotation or comment indicating this is for future use.

Performance Considerations

• ✅ The test uses proper Compose testing patterns with waitForIdle() and scrolling mechanisms
• ✅ GraphQL mutations are properly implemented with coroutines
• ⚠️ The Thread.sleep(5000) in AssignmentsE2ETest.kt adds unnecessary wait time
• ℹ️ Large schema.json file (49K+ lines) may impact build times slightly but is necessary

Security Concerns

• ✅ No hardcoded secrets or tokens
• ✅ Proper authentication flow using test user tokens
• ✅ Uses existing authentication mechanisms from CanvasNetworkAdapter

Test Coverage

✅ Good Coverage

• Assignment list displays discussions with checkpoints correctly
• Checkpoint expand/collapse functionality
• Checkpoint details display (titles, due dates, grades)
• Assignment details page shows checkpoint information
• Page navigation flows

Missing Coverage

• Error handling for failed GraphQL mutations (though there is extensive logging)
• Edge cases: no due date, partial grades, missing checkpoints
• Testing the actual GraphQL creation once the backend bug is fixed

Summary

This is a solid PR that adds necessary E2E test coverage for the Discussion Checkpoints feature. The code follows project conventions and properly uses page objects and test patterns.

Required Changes

1. ❗ Fix hardcoded test data (courseId, courseName, discussionId) - use dynamic data creation
2. ❗ Rename ChekcpointItem.kt to CheckpointItem.kt
3. ❗ Remove unused discussionWithCheckpointsId variable

Recommended Changes

1. Remove or deprecate the unused createDiscussionTopicWithCheckpoints() function
2. Replace Thread.sleep() with proper synchronization mechanisms
3. Clean up commented code once INTEROP-9901 is resolved
4. Add more edge case testing when data seeding becomes available

Low Priority

• Consider adding more accessibility-focused assertions
• Document the schema.json update process for future maintainers

Overall, this PR is in good shape but needs the hardcoded test data issue resolved before merging to ensure test reliability across environments.

---

🤖 Generated with Claude Code

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

PR Review: Discussion Checkpoint E2E Test for Assignment List/Details

Overview

This PR adds E2E test coverage for Discussion Checkpoints functionality in the Student app, along with GraphQL mutations for creating discussions with checkpoints and necessary UI test helpers.

---

✅ Strengths

1. Comprehensive Test Coverage: The new test testDiscussionCheckpointsAssignmentListDetailsE2E provides good end-to-end coverage of the checkpoint feature across assignment list and details pages.

2. Well-Structured Test: The test follows the existing E2E test patterns with proper logging using PREPARATION_TAG, STEP_TAG, and ASSERTION_TAG.

3. Future-Proofing: GraphQL mutation queries are added for eventual automated seeding (INTEROP-9901), showing good forward planning.

4. Good UI Testing Patterns: The page object methods use proper Compose test semantics with test tags and assertions.

---

🔴 Critical Issues

1. Hardcoded Test Data (DiscussionsE2ETest.kt:177-179)

```kotlin
val courseId = 3594441L
val courseName = "Kristof Deak Dedicated Test Course"
val discussionWithCheckpointsId = 22475794L
```

Problem: This is a significant test anti-pattern. The test depends on specific data existing in a shared environment.

Impact:

• Test will fail in other environments
• Not portable or reusable
• Brittle and prone to breaking if data changes
• Violates test isolation principles

Recommendation: Use the seeded data or create test data programmatically. Either:
```kotlin
// Option 1: Use seeded course
val course = data.coursesList[0]
val courseId = course.id
val courseName = course.name

// Option 2: Create discussion programmatically (once INTEROP-9901 is done)
val discussion = DiscussionTopicsApi.createDiscussionTopicWithCheckpoints(...)
```

2. Typo in Filename (ChekcpointItem.kt)

```
libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/ChekcpointItem.kt
```

Problem: The filename is misspelled as "Chekcpoint" instead of "Checkpoint".

Impact:

• Inconsistent naming makes code harder to find
• Potential confusion for future developers
• Affects code navigation and searchability

Recommendation: Rename the file to `CheckpointItem.kt`.

3. Thread.sleep() in Production Test (AssignmentsE2ETest.kt:151)

```kotlin
Thread.sleep(5000) // Give time for the worker to be enqueued
```

Problem: Using fixed sleep timers is unreliable and slows down tests.

Impact:

• Adds unnecessary 5 seconds to every test run
• Can still fail on slower systems if 5 seconds is not enough
• Can pass on faster systems even if 1 second would suffice

Recommendation: Use proper wait/polling mechanisms:
```kotlin
// Use Espresso idling resources or retry with timeout
retryWithIncreasingDelay(times = 10, maxDelay = 5000) {
// Check if worker is enqueued
}
```

---

⚠️ Important Issues

4. Unused Variable (DiscussionsE2ETest.kt:179)

```kotlin
val discussionWithCheckpointsId = 22475794L
```

Problem: This variable is declared but never used in the test.

Recommendation: Remove it or use it if needed for API calls.

5. Missing Column Wrapper Context (DueDateReminderLayout.kt:64)

The PR description mentions "wrap DueDateBlock into a Column to be able to identify it", which is good for testing, but:

Concern: The Column wrapper at line 64 with `testTag("dueDateColumn-${reminderViewState.dueLabel}")` adds semantic grouping. Ensure this does not cause issues with accessibility or screen readers.

Recommendation: Verify with accessibility testing that the `semantics(true) {}` at line 59 does not create unintended screen reader behavior.

6. Error Handling in GraphQL Mutation (DiscussionTopicsApi.kt:113-141)

The verbose error handling with print statements is good for debugging, but:

Concerns:

• Print statements should use a proper logging framework
• Exception stack traces printed to console may clutter test output

Recommendation: Consider using `Log.d()` or a proper logging framework instead of `println()`.

7. Non-Null Assertion Operators (DiscussionTopicsApi.kt:137-139)

```kotlin
if (!errors.isNullOrEmpty()) {
println("GraphQL Errors: ...")
throw Exception("Failed to create discussion: ...")
}
```

Concern: While the null check exists, the error message could be more descriptive.

Recommendation: Include more context like the discussion title and all error details.

---

💡 Suggestions for Improvement

8. Test Tag Naming Convention (AssignmentListPage.kt:146)

```kotlin
hasTestTag("expandDiscussionCheckpoints")
```

Suggestion: Consider more descriptive naming like `"expandDiscussionCheckpointsButton"` or `"discussionCheckpointsExpandIcon"` for clarity.

9. Magic Numbers (DiscussionTopicsApi.kt:86, 92)

```kotlin
pointsPossible = 10.0,
// ...
pointsPossible = 5.0,
```

Suggestion: Consider extracting as constants:
```kotlin
private const val REPLY_TO_TOPIC_POINTS = 10.0
private const val REPLY_TO_ENTRY_POINTS = 5.0
```

10. Test Method Parameters (AssignmentListPage.kt:151)

```kotlin
fun assertDiscussionCheckpointDetails(
additionalRepliesCount: Int,
dueAtReplyToTopic: String,
gradeReplyToTopic: String,
dueAtAdditionalReplies: String = dueAtReplyToTopic,
gradeAdditionalReplies: String = gradeReplyToTopic
)
```

Suggestion: The parameter order mixes data about different checkpoints. Consider grouping or using a data class:
```kotlin
data class CheckpointAssertion(val dueDate: String, val grade: String)
fun assertDiscussionCheckpointDetails(
additionalRepliesCount: Int,
replyToTopic: CheckpointAssertion,
additionalReplies: CheckpointAssertion = replyToTopic
)
```

---

🔍 Code Quality Observations

Positive Patterns:

• ✅ Proper use of test tags throughout for UI testing
• ✅ Consistent use of `useUnmergedTree = true` where needed
• ✅ Good semantic markup with `semantics { heading() }`
• ✅ Follows existing repository patterns and conventions
• ✅ Clear logging in E2E tests for debugging

Schema Changes:

The 79,000+ line change in `schema.json` is expected when regenerating from GraphQL introspection. The formatting change from compact to expanded structure is acceptable if this is the new standard format for the project.

---

🧪 Test Coverage Assessment

Coverage Added: ✅ Good

• Assignment list with checkpoints display
• Checkpoint expansion/collapse
• Checkpoint details (titles, due dates, points, grades)
• Assignment details page checkpoint display

Coverage Gaps:

• No negative test cases (e.g., malformed checkpoint data)
• No test for collapsed state assertions
• No test for checkpoint submission states

---

🎯 Summary

Overall Assessment: This PR adds valuable E2E test coverage for an important feature. The code quality is generally good and follows project conventions, but there are critical issues that must be addressed before merging.

Must Fix Before Merge:

1. ❗ Remove hardcoded test data - use dynamic data seeding
2. ❗ Fix the typo in ChekcpointItem.kt filename
3. ❗ Replace Thread.sleep() with proper wait mechanisms

Should Address:

• Remove unused variable
• Improve error messages
• Consider accessibility impact of semantic changes

Nice to Have:

• Extract magic numbers to constants
• Improve test helper API design
• Use proper logging instead of println

---

Test Execution: The PR description indicates E2E tests have been run successfully ✅

Related Tickets:

• MBL-19335 (this work)
• INTEROP-9901 (future seeding improvements)

Great work on adding comprehensive test coverage for this feature! Please address the critical issues, especially the hardcoded test data, before merging. 🚀

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

Pull Request Review: Discussion Checkpoint E2E Test

Overview

This PR adds E2E tests for discussion checkpoints in the assignment list and details views, along with GraphQL mutation queries for creating discussions with checkpoints. The changes also include UI improvements and test tag additions to support testing.

---

Code Quality & Best Practices

✅ Strengths

1. Well-structured test code: The new E2E test testDiscussionCheckpointsAssignmentListDetailsE2E() follows the existing project patterns with clear PREPARATION/STEP/ASSERTION logging
2. Good use of test tags: Added semantic test tags throughout (e.g., testTag("dueDateColumn-${reminderViewState.dueLabel}")) which improves testability
3. Proper dependency injection: The TestAppManager.kt change correctly uses @Inject for WorkerFactory instead of nullable property
4. GraphQL implementation: The new GraphQL mutations are well-structured with proper error handling

⚠️ Areas for Improvement

1. Typo in filename (libs/pandautils/.../ChekcpointItem.kt:1)

• Issue: Filename is misspelled as ChekcpointItem.kt instead of CheckpointItem.kt
• Impact: Minor - doesn't affect functionality but looks unprofessional
• Recommendation: Rename the file to fix the typo

2. Hardcoded test data (DiscussionsE2ETest.kt:177-180)

val courseId = 3594441L
val courseName = "Kristof Deak Dedicated Test Course"
val discussionWithCheckpointsTitle = "Discussion with Checkpoints"
val discussionWithCheckpointsId = 22475794L

• Issue: Using hardcoded production/QA course IDs and names instead of seeded data
• Impact: High - Test is brittle and depends on external course existing; won't work in other environments
• Recommendation: Use the seeded course from data.coursesList[0] and create the discussion programmatically once GraphQL seeding is available (see commented code on line 186)
• Note: I understand this is a temporary workaround (see INTEROP-9901 comment), but consider adding a TODO or making this more explicit

3. Unused variable (DiscussionsE2ETest.kt:180)

val discussionWithCheckpointsId = 22475794L

• Issue: Variable is declared but never used in the test
• Recommendation: Remove or use it if needed for future assertions

4. Magic numbers in test assertions (AssignmentListPage.kt:163-172, DiscussionsE2ETest.kt:208)

assertCountEquals(2)  // What does 2 represent?
assertDiscussionCheckpointDetails(2, "No due date", ...)  // What's the significance of 2?

• Issue: Unclear what the numbers represent without context
• Recommendation: Extract to named constants or add comments:

private const val CHECKPOINT_COUNT = 2 // Reply to topic + Additional replies
assertDiscussionCheckpointDetails(
    additionalRepliesCount = 2,  // Clear from parameter name
    ...
)

---

Potential Bugs & Issues

🔴 Critical

1. Test relies on external data (DiscussionsE2ETest.kt:173-220)

• Issue: The test depends on a specific course (ID 3594441) existing in the test environment with a pre-created discussion checkpoint
• Risk: Test will fail if:
  • The course doesn't exist in the environment
  • The discussion is deleted or modified
  • Running in a different Canvas instance
• Recommendation: This should be marked as @Ignore or moved to a separate test suite until the GraphQL seeding (INTEROP-9901) is implemented, OR clearly document the setup requirements

🟡 Moderate

2. Potential null safety issue (AssignmentListPage.kt:163)

private fun assertHasAssignmentCommon(..., hasCheckPoints : Boolean = false) {

• Issue: Minor Kotlin style - space before colon in parameter declaration
• Recommendation: Remove space: hasCheckPoints: Boolean = false

3. Schema.json massive diff

• Issue: 49,128 additions and 30,197 deletions in schema.json
• Concern: This appears to be a schema regeneration. Ensure this is intentional and not accidental
• Recommendation: Verify that the schema update is necessary and doesn't introduce breaking changes to existing GraphQL queries

---

Performance Considerations

✅ Good Practices

1. Efficient test structure: Uses waitForIdle() appropriately
2. Proper scrolling: Uses performScrollToNode() instead of scrolling entire list

💡 Suggestions

1. Network timeout in CanvasNetworkAdapter.kt: The 60-second timeout is reasonable for test environments
2. Consider test execution time: The new test enrolls a user and navigates through multiple screens - ensure this doesn't significantly impact E2E test suite duration

---

Security Concerns

✅ No Critical Security Issues Found

Observations:

1. Test code properly uses authentication tokens from seeded data
2. No hardcoded credentials or sensitive data
3. GraphQL mutations include proper error handling
4. The hardcoded course ID is in test code only (though it's still a bad practice for other reasons)

---

Test Coverage

✅ Strengths

1. Comprehensive E2E coverage: New test covers:

   • Assignment list display with checkpoints
   • Expanding/collapsing checkpoint details
   • Navigation to assignment details
   • Checkpoint display on details page

2. Good assertions: Tests verify:

   • Checkpoint titles, due dates, grades
   • Proper UI rendering with test tags
   • Navigation flows

⚠️ Gaps

1. Missing negative test cases

• No tests for error scenarios (e.g., what if checkpoint data is malformed?)
• No tests for edge cases (e.g., checkpoints with past due dates, no points possible)

2. GraphQL mutation not tested

• The createDiscussionTopicWithCheckpoints() function includes extensive debug logging (lines 117-133) but isn't actually used in tests yet
• Recommendation: Once INTEROP-9901 is resolved, add tests that verify the GraphQL mutation works correctly

3. UI state tests

• Consider adding unit/integration tests for the checkpoint UI components (CheckpointItem.kt, DueDateReminderLayout.kt)
• The androidTest files in pandautils (lines 15-17 of diff) only show minor updates

---

Additional Observations

📝 Minor Issues

1. Inconsistent spacing (DueDateReminderLayout.kt:64)

Column(modifier = Modifier.testTag("dueDateColumn-${reminderViewState.dueLabel}")) {

• Consider adding the testTag as a separate modifier call for readability:

Column(
    modifier = Modifier
        .testTag("dueDateColumn-${reminderViewState.dueLabel}")
) {

2. Comment placement (DiscussionsE2ETest.kt:185-186)

• The commented-out GraphQL mutation code should either be removed or have a more prominent TODO comment explaining when it will be uncommented

3. Test extends different base class (DiscussionsE2ETest.kt:40)

• Changed from StudentTest() to StudentComposeTest()
• Ensure this is intentional - verify that StudentComposeTest provides the necessary setup for both existing and new tests in this class

---

Summary & Recommendations

Priority Actions

1. 🔴 HIGH: Address the hardcoded test data issue - either fully seed the data or clearly document the manual setup required
2. 🟡 MEDIUM: Fix the typo in ChekcpointItem.kt filename
3. 🟡 MEDIUM: Remove unused discussionWithCheckpointsId variable
4. 🟡 MEDIUM: Verify the schema.json changes are intentional and complete

Overall Assessment

This is a solid PR that adds valuable E2E test coverage for discussion checkpoints. The code follows project conventions well, and the test infrastructure improvements (test tags, compose support) are excellent. The main concern is the reliance on hardcoded test data, which should be addressed before merging or clearly documented as a known limitation until the GraphQL seeding feature is available.

Recommendation: ✅ Approve with minor changes - Address the hardcoded data concern (either fix or document clearly), fix the filename typo, and this PR is good to merge.

---

Review completed by Claude Code

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[claude]

PR Review: Discussion Checkpoint E2E Test for Assignment List/Details

Summary

This PR adds E2E tests for discussion checkpoints in assignment lists and details, along with GraphQL mutation queries for creating discussions with checkpoints. The changes primarily involve test infrastructure and test page objects.

---

✅ Strengths

1. Comprehensive Test Coverage: The new E2E test testDiscussionCheckpointsAssignmentListDetailsE2E provides thorough coverage of the discussion checkpoints feature across multiple screens.

2. Well-Structured Test: The test follows a clear Arrange-Act-Assert pattern with excellent logging using PREPARATION_TAG, STEP_TAG, and ASSERTION_TAG.

3. Good Testability Improvements: Added test tags to UI components (e.g., testTag("dueDateColumn-${reminderViewState.dueLabel}")) which is excellent for E2E testing.

4. GraphQL Integration: Added proper GraphQL mutation files with appropriate schema updates for discussion checkpoint creation.

5. Dependency Injection Improvement: The TestAppManager.kt change properly uses @Inject annotation for the WorkerFactory instead of nullable with fallback, which is cleaner.

---

🔍 Issues & Concerns

Critical Issues

1. Hardcoded Test Data (DiscussionsE2ETest.kt:177-180)

   val courseId = 3594441L
   val courseName = "Kristof Deak Dedicated Test Course"
   val discussionWithCheckpointsTitle = "Discussion with Checkpoints"
   val discussionWithCheckpointsId = 22475794L

   Problem: This test depends on a specific pre-existing course/discussion that may not exist in all test environments. This makes the test non-portable and fragile.

   Recommendation: Either:

   • Use the commented-out GraphQL seeding approach when INTEROP-9901 is resolved
   • Create the course/discussion dynamically in the test setup
   • Document this as a known limitation in the test or PR description

   Risk: High - Test will fail in any environment without this specific course.

2. Unused Variable (DiscussionsE2ETest.kt:180)

   val discussionWithCheckpointsId = 22475794L

   Problem: This variable is declared but never used in the test.

   Recommendation: Remove if not needed, or use it if it was intended for future assertions.

3. Typo in Filename (ChekcpointItem.kt)

   libs/pandautils/src/main/java/com/instructure/pandautils/compose/composables/ChekcpointItem.kt
   

   Problem: File is named ChekcpointItem.kt instead of CheckpointItem.kt (missing 'h').

   Recommendation: Rename the file to match the class name CheckpointItem.

   Impact: While Kotlin doesn't require matching filenames, this violates naming conventions and could cause confusion.

Code Quality Issues

4. Missing Space in Comment (AssignmentListPage.kt:165)

   // Check if the assignment is a discussion with checkpoints, if yes, we are expecting 2 due dates for the 2 checkpoints.

   The variable name hasCheckPoints has inconsistent capitalization (should be hasCheckpoints with lowercase 'p').

5. Inconsistent Test Tag Naming (CheckpointItem.kt:72)

   .testTag("checkpointDueDate_${discussionCheckpointUiState.name}")

   Test tags use different separators: dueDateColumn- (hyphen) vs checkpointDueDate_ (underscore).

   Recommendation: Standardize on one separator throughout the codebase.

6. Complex Compose Test Selectors (StudentAssignmentDetailsPage.kt:71)

   composeTestRule.onNode(hasText(dueAt) and hasAnyAncestor(hasTestTag("dueDateColumn-$checkpointText") and hasAnyDescendant(hasTestTag("dueDateHeaderText-$checkpointText")))).assertIsDisplayed()

   Problem: This selector is extremely complex and fragile. If the UI hierarchy changes, this will break.

   Recommendation: Consider adding a more specific test tag directly to the due date text element.

Potential Bugs

7. Formatting Issue (AssignmentListPage.kt:163)

   private fun assertHasAssignmentCommon(assignmentName: String, assignmentDueAt: String?, expectedLabel: String? = null, hasCheckPoints : Boolean = false) {

   Problem: Extra space before colon in hasCheckPoints : Boolean.

   Recommendation: Change to hasCheckPoints: Boolean.

8. Missing Teacher Variable (DiscussionsE2ETest.kt:175-176)
   The test seeds a teacher (teachers = 1) but never uses it. The commented GraphQL approach (line 186) references teacher.token which isn't available.

   Recommendation:

   val teacher = data.teachersList[0]

   And uncomment/fix line 186 when INTEROP-9901 is done.

Test Robustness

9. Magic Numbers (AssignmentListPage.kt:171)

   true).assertCountEquals(2)

   The hardcoded 2 assumes exactly 2 checkpoints. If this becomes configurable, the test will be brittle.

   Recommendation: Consider making this a parameter or constant.

10. Error Handling in GraphQL (DiscussionTopicsApi.kt:113-140)
    Good: The error handling with detailed logging is excellent for debugging.

    Concern: The function doesn't return anything, making it hard to verify success or get the created discussion ID.

    Recommendation: Return the created discussion topic data for verification in tests.

---

🔒 Security Concerns

• No security issues identified. The changes are test-only code that doesn't expose any sensitive data or create security vulnerabilities.

---

⚡ Performance Considerations

1. Schema.json Size: The schema file went from ~30K to ~49K lines. While this is auto-generated, ensure the GraphQL code generation step doesn't significantly slow down builds.

2. Test Execution Time: The new E2E test makes multiple API calls and UI interactions. Consider:

   • Running this test in a separate suite if it's slow
   • Using test tags appropriately (already done with @E2E and @TestMetaData)

---

📝 Test Coverage

Strong Points:

• Tests both assignment list and assignment details views
• Verifies checkpoint expansion/collapse functionality
• Checks grade display, due dates, and point values
• Uses proper test categorization with Priority.IMPORTANT

Gaps:

• No negative test cases (what if checkpoints fail to load?)
• No tests for checkpoint interactions (submitting, grading)
• No accessibility testing for checkpoint UI

---

🎯 Recommendations Summary

Must Fix:

1. Address hardcoded test data dependency (courseId, courseName, etc.)
2. Remove unused discussionWithCheckpointsId variable
3. Rename ChekcpointItem.kt → CheckpointItem.kt
4. Fix spacing in hasCheckPoints : Boolean

Should Fix:
5. Simplify complex Compose selectors
6. Add teacher variable for future GraphQL usage
7. Standardize test tag naming conventions
8. Return data from createDiscussionTopicWithCheckpoints()

Nice to Have:
9. Add error handling tests
10. Document the temporary test data dependency
11. Add TODO comments linking to INTEROP-9901 ticket

---

📊 Overall Assessment

Code Quality: ⭐⭐⭐⭐☆ (4/5)

• Well-structured tests with good practices
• Minor issues with hardcoded data and naming

Test Coverage: ⭐⭐⭐⭐☆ (4/5)

• Comprehensive happy path coverage
• Missing edge cases and error scenarios

Maintainability: ⭐⭐⭐☆☆ (3/5)

• Hardcoded test data significantly impacts maintainability
• Complex selectors may be brittle

Security: ⭐⭐⭐⭐⭐ (5/5)

• No security concerns

Recommendation: ✅ Approve with changes

This PR adds valuable test coverage for discussion checkpoints. The main blocker is the hardcoded test data dependency which should be addressed or clearly documented before merging. The other issues are minor and can be fixed quickly.

---

Great work on the comprehensive test implementation! 🚀

[inst-danger]

Parent Install Page

[claude]

PR Review: Discussion Checkpoint E2E Test for Assignment List/Details

Summary

This PR adds comprehensive E2E test coverage for Discussion Checkpoints in the Assignment List and Details views. The implementation includes GraphQL mutation queries for data seeding, UI test enhancements, and necessary component modifications to support checkpoint testing.

✅ Strengths

Test Structure

• Well-organized test flow with clear preparation, execution, and assertion phases
• Comprehensive assertions covering both assignment list view and details page
• Good use of logging with PREPARATION_TAG, STEP_TAG, and ASSERTION_TAG

Code Quality

• Proper test tags for UI testing (dueDateColumn, dueDateHeaderText)
• Semantic improvements: Wrapping DueDateBlock in Column improves testability
• GraphQL preparation for future use (INTEROP-9901)

⚠️ Issues & Concerns

🔴 Critical: Hard-coded Test Data

The most significant issue is hard-coded course and discussion IDs at DiscussionsE2ETest.kt:177-180

Problems:

1. Test fragility - depends on external data that may not exist in all environments
2. Environment coupling - hardcoded to specific test environment
3. Maintenance burden - test fails if course/discussion is deleted
4. Inconsistent pattern - other tests properly seed their own data
5. Unused variable: discussionWithCheckpointsId is declared but never used

Recommendation: Once INTEROP-9901 is resolved, replace with proper data seeding using createDiscussionTopicWithCheckpoints()

🟡 Minor Issues

1. Typo in Filename: ChekcpointItem.kt should be CheckpointItem.kt (missing p)
2. Large Schema File Update: +49,128 / -30,197 lines - verify this was intentional
3. Test Base Class Change: StudentTest() to StudentComposeTest() - appears correct for Compose testing

🎯 Best Practices Observed

1. Consistent naming conventions following Kotlin standards
2. Good separation of concerns with page objects pattern
3. Reusable assertion methods like assertDiscussionCheckpointDetails()
4. Proper use of @TestMetaData with SecondaryFeatureCategory.DISCUSSION_CHECKPOINTS

🔒 Security Review

• No security concerns identified
• Proper authentication through tokenLogin()

⚡ Performance Considerations

• Large schema file will increase repository size
• Hard-coded course dependency may cause test flakiness

📊 Test Coverage

Good E2E coverage:

• ✅ Assignment list view with checkpoints
• ✅ Expand/collapse checkpoint details
• ✅ Assignment details page
• ✅ Multiple checkpoints with grades and due dates

📝 Recommendations

Immediate Actions

1. Address hard-coded test data - add TODO comment linking to INTEROP-9901
2. Remove unused variable discussionWithCheckpointsId at line 180
3. Add comment explaining large schema.json update

Future Improvements

1. Rename ChekcpointItem.kt to CheckpointItem.kt
2. Replace hard-coded data once INTEROP-9901 is complete

Overall Assessment

Status: ⚠️ Approve with Concerns

Solid E2E test work following established patterns with valuable test coverage. Main concern is hard-coded test data dependency that should be addressed.

Great work on comprehensive test coverage! 🎉

---

Generated with Claude Code - Automated PR Review

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[claude]

Pull Request Review: Discussion Checkpoint E2E Test

This PR adds E2E test coverage for discussion checkpoints in the assignment list and details views.

--- STRENGTHS ---

1. Comprehensive Test Coverage: The new E2E test provides good coverage for discussion checkpoints across multiple screens.

2. Good Code Organization: The changes follow the existing test patterns and page object model structure.

3. Clear Logging: Excellent use of logging tags makes the test flow easy to follow and debug.

4. Test Infrastructure: Added GraphQL schema and mutation queries for future checkpoint creation.

5. Accessibility: Proper use of test tags and semantic modifiers in UI components.

--- CRITICAL ISSUES ---

1. HARDCODED TEST DATA (DiscussionsE2ETest.kt:177-180)

   • Test relies on hardcoded course/discussion IDs (courseId = 3594441L, courseName = "Kristof Deak Dedicated Test Course")
   • Makes the test non-portable and will fail in other environments
   • discussionWithCheckpointsId variable is declared but never used
   • RECOMMENDATION: Either seed the discussion programmatically or document environment prerequisites

2. TYPO IN FILENAME

   • File is named ChekcpointItem.kt instead of CheckpointItem.kt (missing p)
   • RECOMMENDATION: Rename the file

3. COMMENTED CODE (DiscussionsE2ETest.kt:185-186)

   • Test comment references using course.id and teacher.token, but test uses hardcoded course
   • RECOMMENDATION: Update the comment

--- MODERATE ISSUES ---

4. TEST USES WRONG DATA SOURCE (DiscussionsE2ETest.kt:175-183)

   • Seeds data with seedData() but then ignores it and uses hardcoded course
   • Seeded teacher is not used
   • RECOMMENDATION: Either use seeded course or remove unnecessary seeding

5. MAGIC NUMBERS (AssignmentListPage.kt:208)

   • Number 2 is hardcoded without context
   • RECOMMENDATION: Use named constant EXPECTED_CHECKPOINT_COUNT = 2

--- MINOR ISSUES ---

6. Missing unit tests for createDiscussionTopicWithCheckpoints method
7. Large schema file (49k additions, 30k deletions) - confirm this is expected
8. Potential null safety issue with test tags

--- SECURITY & PERFORMANCE ---

Security: No concerns identified
Performance: Test enrolls student in existing course (may have implications if course is large)

--- TEST COVERAGE ---

Good: E2E test covers main user flow, tests both list and detail views
Missing: Unit tests for GraphQL method, error case tests, UI component tests

--- RECOMMENDATIONS BEFORE MERGE ---

1. Fix hardcoded test data issue (critical)
2. Remove unused discussionWithCheckpointsId variable
3. Fix filename typo ChekcpointItem.kt
4. Clean up unnecessary data seeding

--- SUMMARY ---

Solid PR that adds valuable E2E test coverage. Main blocker is the hardcoded test data which makes the test environment-dependent. Fix this and the filename typo before merging.

Overall Assessment: NEEDS CHANGES

Generated with Claude Code

[inst-danger]

Parent Install Page

[inst-danger]

Teacher Install Page

[inst-danger]

Student Install Page

[Copilot]

Pull Request Overview

This PR adds E2E test support for discussion checkpoint functionality in the assignment list and details views. The changes primarily focus on improving test infrastructure and adding test tag identifiers to enable automated testing of discussion checkpoints.

Key changes:

• Added test tags and identifiers to discussion checkpoint UI components for E2E testing
• Implemented GraphQL mutation support for creating discussion topics with checkpoints
• Created new E2E test for discussion checkpoints on assignment list and details pages

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
GradesScreen.kt	Updated test tag from singular to plural form for consistency
AssignmentListScreen.kt	Updated test tag from singular to plural form for consistency
DueDateReminderLayout.kt	Wrapped DueDateBlock in Column with test tag and improved test tag specificity
ChekcpointItem.kt	Added test tags to checkpoint name and made due date test tag more specific
GradesScreenTest.kt	Updated test assertions to use new checkpoint-specific test tags
AssignmentListScreenTest.kt	Updated test assertions to use new checkpoint-specific test tags
AssignmentListPage.kt	Added methods for checkpoint interaction and assertion in E2E tests
AssignmentDetailsPage.kt	Changed composeTestRule visibility from private to public
TestMetaData.kt	Added DISCUSSION_CHECKPOINTS secondary feature category
CanvasNetworkAdapter.kt	Added DateTime scalar adapter for GraphQL
DiscussionTopicsApi.kt	Added method to create discussion topics with checkpoints via GraphQL
CreateDiscussionTopicWithCheckpoints.graphql	New GraphQL mutation for creating discussion topics with full configuration
CreateDiscussionTopicMinimal.graphql	New GraphQL mutation for creating discussion topics with minimal configuration
StudentAssignmentDetailsPage.kt	Added assertion method for checkpoint details on assignment details page
DashboardPage.kt	Added selectCourse method accepting course name string
DiscussionsE2ETest.kt	Added new E2E test for discussion checkpoints and fixed typo in comment

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SimpleDateFormat is not thread-safe. Since this adapter may be used in concurrent contexts, consider using DateTimeFormatter from java.time package or synchronizing access to SimpleDateFormat.

[kdeakinstructure]

This pull request content is already merged within this pr: #3344

So closing this PR as it became irrelevant.