Skip to content

Conversation

rh12
Copy link
Contributor

@rh12 rh12 commented Oct 17, 2024

refs: MBL-17976
affects: Student, Teacher, Parent
release note: none

Because it was not reproducible, this is a best effort attempt to fix the crash during CalendarFilter load.

Problem

CDCalendarFilterEntry is sometimes stored without a valid contextID, which results in a crash while force unwrapping CDCalendarFilterEntry.context. We assume that this may be a result of some old ID format which we are not aware of, but still used in some accounts.

  • Removing the force unwrap would have required extensive changes and complications.
  • Providing a default value is also difficult, because there is no meaningful fallback context here.
  • Silently ignoring the filters with invalid (which means empty) contextIDs seemed to be the lesser evil here, especially that a PTR should fix any missing calendars.

What's changed

  • CalendarFilters with invalid contextIDs already stored in CoreData are not displayed
  • CalendarFilters with invalid contextIDs coming from backend are not saved to CoreData
  • invalid contextIDs during save are logged as errors: "CDCalendarFilterEntry save failed with invalid contextId"
  • invalid contextIDs during Context init are logged as errors: "Context created with invalid contextId"
  • failed ID inits (failing to decode as Int or String) are logged as errors: "Empty ID decoded from unhandled data"

Test Plan

Smoke test Calendar Filters in all 3 apps.

Checklist

  • Follow-up e2e test ticket created
  • Tested on phone
  • Tested on tablet

rh12 added 5 commits October 16, 2024 14:21
refs: MBL-17976
affects: Student, Teacher
release note: none
refs: MBL-17976
affects: Student, Teacher, Parent
release note: none
@rh12 rh12 self-assigned this Oct 17, 2024
@rh12 rh12 requested review from vargaat and szabinst as code owners October 17, 2024 07:37
@inst-danger
Copy link
Contributor

inst-danger commented Oct 17, 2024

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Oct 17, 2024

Student Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Oct 17, 2024

Warnings
⚠️ This pull request will not generate a release note.
⚠️ One or more files are below the minimum test coverage 50%

Affected Apps: Student, Teacher, Parent

MBL-17976

Coverage New % Master % Delta
Canvas iOS 91.2% 91.18% 0.02%
Core/Core/Planner/CalendarEvent/Model/API/PostCalendarEventRequest.swift 0% 0% 0%
Core/Core/Planner/CalendarEvent/View/CustomFrequencyComponents/OccurrencesCountInputDialog.swift 0% 0% 0%
Core/Core/Planner/CalendarEvent/Model/Helpers/RecurrenceRule+SelectionDescription.swift 43.33% 43.33% 0%
Core/Core/Grades/View/CustomSwipeAction.swift 0% 0% 0%

Generated by 🚫 dangerJS against f884e88

@rh12
Copy link
Contributor Author

rh12 commented Oct 17, 2024

Parent Build QR Code:

Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

+1

@rh12
Copy link
Contributor Author

rh12 commented Oct 17, 2024

@suhaibabsi-inst

+1

Sorry, is this an approval? Did you also test them?

@suhaibabsi-inst
Copy link
Contributor

No haven't QA tested it. Need some time.

@rh12
Copy link
Contributor Author

rh12 commented Oct 17, 2024

No haven't QA tested it. Need some time.

Sure, no worries, take your time. I just wasn't sure which stage is it.

Copy link
Contributor

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

@rh12 rh12 merged commit ac89807 into master Oct 21, 2024
6 checks passed
@rh12 rh12 deleted the bugfix/MBL-17976-Calendar-Filter-crash branch October 21, 2024 10:17
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