loyalty-card-locker icon indicating copy to clipboard operation
loyalty-card-locker copied to clipboard

Initial pkpass support

Open TheLastProject opened this issue 5 years ago • 7 comments

This will probably take a bit longer to make completely useful, but this at least does the initial parsing. Marking this WIP just because I believe in sharing progress to allow for early feedback.

When this is done, it will fix #309.

TODO:

  • [x] Parses https://github.com/keefmoon/Passbook-Example-Code/blob/master/Pass-Example-Generic/Pass-Example-Generic.pkpass successfully
  • [x] Unit tests
  • [x] Make unit tests directly work with pkpass files (for more easy extending in the future)
  • [X] Parsing relevant extra fields
  • [x] i18n support
  • [ ] Open .pkpass files from browser

This change is Reviewable

TheLastProject avatar Dec 09 '19 16:12 TheLastProject

Documentation: https://developer.apple.com/library/archive/documentation/UserExperience/Reference/PassKit_Bundle/Chapters/TopLevel.html#//apple_ref/doc/uid/TP40012026-CH2-SW1

TheLastProject avatar Dec 09 '19 16:12 TheLastProject

I've opted for using a new EXTRAS database field to store this all in, because putting all of this data in the note doesn't seem doable. See https://github.com/brarcher/loyalty-card-locker/issues/309#issuecomment-565748328 for how this looks to the end-user.

TheLastProject avatar Dec 15 '19 14:12 TheLastProject

i18n is working, but I need to fix all the tests still:

photo_2019-12-15_19-11-24

TheLastProject avatar Dec 15 '19 18:12 TheLastProject

While not perfect, it does enough to be useable. I guess I consider this to be "ready for review" now, @brarcher.

It's a rather huge change, so to explain the architecture a bit:

  • There's a new ExtrasHelper which stores "extra" data. The reason for this is that (1) note doesn't support i18n, (2) there's simply way too much data to store in a note and still have a decent user experience
  • The ExtrasHelper saves to the database as JSON and parses the JSON from the database. This is on purpose because (1) to my knowledge JSON by itself does not have code execution exploit risks like other ways of storing data does, (2) it makes it easy to share it with the existing share feature and (3) there are no real guarantees as to what fields there will be which makes it difficult to create a good DB structure for

Some notes:

  • I'm honestly not happy with the whole codebase being filled with throws JSONException now, but I'm not sure how else to make this work...
  • Android intents are very confusing but this at least consistently works for downloaded .pkpass files in a file manager.
  • We lack a proper i18n chain. Right now it's just user's preferred language -> en -> untranslated. This means that if Android returns es_AR for example, we'll likely fail to return the es translation and will instead return the en one. I do not yet know how to properly fix this.

TheLastProject avatar Dec 16 '19 16:12 TheLastProject

Screenshot_1576667915

I couldn't resist and I'm sure the legal worries of #303 don't apply when they literally put the icon in the file that's provided.

TheLastProject avatar Dec 18 '19 11:12 TheLastProject

I suppose that the name of the app should be changed, as boarding cards or event tickets are not loyalty cards. I am going to open a new issue about that.

airon90 avatar Feb 22 '20 08:02 airon90

I personally would keep the app name the same and just write in the description it also supports event tickets and stuff.

TheLastProject avatar Feb 22 '20 14:02 TheLastProject