io7m / android Goto Github PK

Just need to verify that the removal of CatalogLeftPaddedButton isn't going to cause issues on the old devices.

It has some dependencies on some code that needs to be removed and/or modified. The card creator appears to be disabled in the code currently, so if it's not going to be used, I'll delete it (that way I don't need to update it to accomodate the account changes).

The current code scans OPDS feeds one entry at a time and ends up using whichever DRM licensor it encounters last. The code should instead be storing a list of the licensors it encounters: A silent assumption that all DRM licensors in a feed are the same licensor sounds like an incredibly nasty bug to track down when that assumption later turns out to be incorrect.

It shows up as the offensive magenta coloured text that I picked to make sure that unthemed items were extremely apparent.

We need a set of Android Studio rules that can be applied across the codebase and that will automatically be found by any new developers.

The current application abuses the books API by simply deleting and recreating the book controller every time an account changes. This has the side effect of leaking a couple of thread pools in the process.

The right way to do this from the start would have been to change the books API to make it understand the concept of multiple accounts and profiles.

This is the bulk of the work required to get the application to work with profiles. Need to document the way that data is currently stored on the device, the way that it will be stored with multiple accounts and profiles, and write a basic migration strategy to update the stores of existing users.

This is just paranoia, but any activity that knows about "the current profile" should be closed when a ProfileSelected event is published. This ensures that the user can't go back to the activities of a previous profile using the back button.

The code creates instances of AccountAuthenticationProvider in many places, and actually saves them to the device, but they are completely unused anywhere.

The org.nypl.simplified.app.reader.ReaderBookmarks type uses Android's SharedPreferences to store bookmarks. This was fine for the old single-account no-profiles case, but bookmarks now need to be stored in a profile-specific persistent store.

The previously immutable AccountCredentials type has been transformed into a mutable type with six Option fields. Those fields have causal relationships between each other; when you have one of them, you must also have at least two more, but as they're encoded as a pile of unrelated optional fields, the type system can't tell you this and can't enforce that fact. This leads to a lot of unpleasant checks throughout the code, and it's unclear without a detailed study of the execution path of the program when some fields can be set and not others. I've managed to work out the relationships, so this type needs to be completely replaced with an immutable credentials type as part of the accounts/profiles update.

A logo is needed for the app and the splash screen.

The org.nypl.simplified.books.core package is rather large. It would be better to split the core package into multiple subpackages to make it slightly easier to digest.

Someone changed how OPDS availability is calculated and didn't bother to update any of the unit tests. Now, all of the OPDS availability-related unit tests fail and it's not clear if it's because the code is wrong, or because the OPDS entry specimens need to be changed to match what's actually served now.

There should be a section in the Settings section that shows the Git commit that was used to produce the build.

Here's one way to do it:

https://mrhaki.blogspot.co.uk/2015/04/gradle-goodness-use-git-commit-id-in.html

Right now if a book operation (borrowing, for example) is in progress, clicking another book button in the UI will enqueue an operation but this won't be reflected in the UI right away. The reason for this is that state changes are published when the background task starts, and the task may not start for a while if another task is already in progress.

Easy fix: Publish book state changes to the registry immediately (such as setting the state of a book to "download requested") and then let the background tasks update those states when the tasks start running.

Right now, all over the project, activities are starting new activities by calling startActivity on themselves. Need to replace all startActivity calls with a call to a method defined in the project so that we can be confident that activities are being started with the right flags (such as the flag that prevents the user going back to previous activities with the back button).

Right now, the unpacking step is done manually.

The stock DatePicker that comes with Android is hilariously broken on some devices:

Replace it with a custom view that contains three number spinners (year/month/day).

It no longer makes sense for different accounts to get different action bar and text colors. Any reference to mainColor from the account provider configuration should be audited and most likely removed.

The code is already there, just need to actually call the confirmation dialog code from the navigation drawer activity.

Right now, if an account/profile exists with an account provider that has been removed, any attempt to open the profiles database will result in an exception. This exception is propagated to the main thread and as such the app will crash.

I'm not exactly sure what would be a reasonable way to handle a failure here; not being able to open the profiles database is not something that the app can recover from, but at the same time, perhaps the above shouldn't be a fatal error. Removing an account provider is something that can only occur by modifying the app and recompiling it, so it's hardly urgent, but it is something that will need to be handled in future once the account providers come from a remote server.

Currently, many tests fail. Many tests aren't run as part of the build.

Additionally, the tests are structured such that the junit4 and android frontends for each set of tests both delegate to implementations that don't refer to junit at all. This used to be necessary because there was no way to write tests such that they could be executed both locally (for quick testing) and/or on a real device (for testing in an Android environment). This is no longer necessary: Android can now run JUnit 4 tests and the code required to support both can be essentially thrown away.

Ripping out the existing account handling with regards to branding also removed the logic that picks an ActionBar or NoActionBar theme variant. Putting that back in is trivial, but the original sources need to be inspected to determine which activities had action bars and which didn't.

This same bit of code has been copied and pasted repeatedly throughout the codebase:

    final AccountAuthenticationCredentials credentials = this.getAccountCredentials();
    final AccountBarcode barcode = credentials.getBarcode();
    final AccountPIN pin = credentials.getPin();

    HTTPAuthType auth = HTTPAuthBasic.create(barcode.toString(), pin.toString());
    if (credentials.getOAuthToken().isSome()) {
      final HTTPOAuthToken token = ((Some<HTTPOAuthToken>) credentials.getOAuthToken()).get();
      if (token != null) {
        auth = HTTPAuthOAuth.create(token);
      }
    }

    return auth;

This needs to go in a method somewhere instead of being endlessly replicated.

The current splash screen needs replacing with one that leads to the profile selection screen (or straight to the catalog if the anonymous profile is enabled).

Various bits of code create values of AccountPatron, but nothing ever reads them.

See #9

When the user is idle in any activity other than the reader, log them out after 10 minutes of inactivity.

The various subtypes extend BookBorrowException, not BookRevokeException.

Add an item in the navigation menu/drawer that returns the user to the profile selection screen.

The profiles view is currently using icons from OpenIconic:

Some activities use the default animation, some use no animation. Most use no animation on creation but then use the default animation on closing.

Make this consistent!

Related to #16

Is it OK to use Java 8 APIs?

Download cancellation hasn't been implemented yet.

A logo is needed for the navigation drawer and for the account settings.