Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve PDF annotations even when cache is reset #2965

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

suhaibabsi-inst
Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst commented Nov 11, 2024

refs: MBL-18069
affects: Student
release note: PDF annotations is preserved even when cache is reset

Test Plan

  • Login to Student app
  • Open a PDF file and make some annotations to it, then go back.
  • Close the app entirely.
  • Go to the App's page on Settings app.
  • Turn on "Reset cache on next launch" toggle.
  • Launch the app, then login back to the same user.
  • Open the PDF file that was annotated in step 2.
  • You should see the previously saved annotations on that file.

Video Record

Recording covering PDF annotation persistence scenarios, which are:

  • On logout
  • On cache reset.
pdf_annotations_saving.mov

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-18069
affects: Student
release note: PDF annotations is preserved even when cache is reset

test plan: See PR description
@inst-danger
Copy link
Contributor

inst-danger commented Nov 11, 2024

Student Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Nov 11, 2024

Fails
🚫 Build failed, skipping coverage check

Release Note:

PDF annotations is preserved even when cache is reset

Affected Apps: Student

MBL-18069

❌ Swift lint
----- xcbeautify -----
Version: 2.11.0
----------------------

❌ /Users/vagrant/git/Core/Core/Login/LoginSession.swift:177:1: Trailing Whitespace Violation: Lines should not have trailing whitespace (trailing_whitespace)

Generated by 🚫 dangerJS against cb51a1c

@suhaibabsi-inst suhaibabsi-inst marked this pull request as ready for review November 13, 2024 08:55
ndrsszsz
ndrsszsz previously approved these changes Nov 14, 2024
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
Tested on iPhone 11, iOS 18.1

Core/Core/Login/LoginSession.swift Outdated Show resolved Hide resolved
Core/Core/Login/LoginSession.swift Outdated Show resolved Hide resolved

try? fileManager
.contentsOfDirectory(at: folderUrl, includingPropertiesForKeys: nil)
.filter({ $0.lastPathComponent.hasPrefix(URL.Paths.offline) == false })
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the filtering here. My concern with this is that in case we want to save something in the user's session folder in the future it will be copied to the annotated pdf directory since this only excludes the offline folder.
All the annotated pdfs are saved in directories with only numbers in their names and the annotated files have a pdf extension. Could we use this knowledge to move only such directories?

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