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

Observations list page #118

Closed
mstenta opened this issue Apr 11, 2018 · 6 comments
Closed

Observations list page #118

mstenta opened this issue Apr 11, 2018 · 6 comments

Comments

@mstenta
Copy link
Member

mstenta commented Apr 11, 2018

Below is a rough idea for Observations list page, which I envision would be the first thing you see when you open the app. Here is a mockup as a JSFiddle: https://jsfiddle.net/4nL9khx3/21

  • Table with columns:
    • Date/time
    • Log name
    • Sync status (“unsynced”, spinner/throbber while syncing, or date that it was synced)
    • “X” to delete (with confirm “Are you sure?” popup)
  • Button above table: “Sync all to farmOS”. When clicked:
    • Test connection to farmOS server
      • If offline, show message saying that no connection could be made.
      • Show login prompt, if necessary
        • If login credentials are not stored, or:
        • If login fails with 403 (show message indicating that credentials may be incorrect)
    • Iterate through all unsynced logs
      • Change “Sync status” from “unsynced” to spinner/throbber
      • Send log via AJAX to server
      • If successful:
        • Record timestamp that the log was synced
        • Store the remote log ID (from Drupal)
        • Clear all information from the locally stored log except for the log timestamp, name, synced timestamp, and remote ID
        • Change spinner/throbber to the Date/time that the log was synced, linked to the log URL in the remote farmOS site
  • Button above table: “Create observation” - sends you to new observation form
@mstenta
Copy link
Member Author

mstenta commented Apr 11, 2018

The existing observation form is a good start, here is how I see that tying into the above:

  • New observation form
    • Fields
      • Log name
      • Notes
      • Photo(s)
      • Area reference(s)
      • Quantity measurement(s)
    • “Save” button - when clicked:
      • Add new log to list of observations, with “unsynced” status
      • Show observations list

@jgaehring
Copy link
Member

jgaehring commented Apr 27, 2018

Apologies for posting such a novel here, but these are my running notes as I've been reviewing Alex's latest commits to farmos-native and trying to map out our next stages of development.

Modelling the Observations list

I want to separete our model for the Observations list into the constituent models for the the client and data plugin which will handle the rendering and HTTP requests, respectively. In doing so, I'm hoping it will reflect a model that's more idiomatic to Vue and single page applications in general. Note that currently, the db module and http module are actually lumped together into one big Vuex module (dataModule.js), which is already over 400 lines of code, so I want to separate that out before starting on the Observations list.

(For reference, I've added a rough overview in the docs hightlighting how the app is structured around Vue's and Vuex's API's., which might help explain some of the rationale for what follows.)


Editted: in order the reflect changes in #3 (comment)

AllObservations Component

  • User requests to see 'All Observations' (or opens app for 1st time)
    • Routes to AllObservations.vue (or loads it as a nested component)
    • Load cached logs into state, filtering for observations only
  • Render AllObservations.vue page/component
    • Render a table with table headings: Date/time, Log Name, Sync Status, Delete
    • Iterate over each log; for each log, render a row and map its state to the corresponding column:
      • Date/Time -> log[i].date
      • Log Name -> log[i].name
      • Sync status, if:
        • (!log[i].isReadytoSync && !log[i].isSynced)-> "unsynced" (and/or a toggle, to change state to "Ready"?)
        • (log[i].isReadytoSync && !log[i].isSynced) -> spinner/throbber
        • (log[i].isSynced) -> log[i].timestamp
        • “X” to delete, when clicked, call openDeleteDialog(i)
        • Edit/View, if:
          • !log[i].isSynced -> "Edit" button, which routes to NewObservation.vue
          • log[i].isSynced -> "View" button, with a link to web farmOS
      • Render a button above table: “Sync all to farmOS”. When clicked:
        • Commit a mutation to setAllLogsToReady
      • Render a button above table: “Create observation”. When clicked:
        • Routes to NewObservation.vue (or loads it as a nested component)
      • Conditionally render a warning modal, only if showDeleteDialog === true:
        • Render the message: `Are you sure you want to permanently delete the log "${logs[logIndexToDelete].name}"?`
        • Render a "Yes, delete it" button, which calls confirmDelete()
        • Render a "No, don't delete it" button, which calls cancelDelete()
      • For the Delete Dialog:
        • Provide some local component state:
          • showDeleteDialog, set to false by default
          • logIndexToDelete, set to null by default
        • Provide a component method, openDeleteDialog, which
          • takes the index of a log as argument
          • sets showDeleteDialog to true
          • sets logIndexToDelete equal to the index argument
        • Provide a component method, confirmDelete, which
          • dispatches the deleteLog action to the store, passing logIndexToDelete as payload
          • sets showDeleteDialog back to false and logIndexToDelete to null
        • Provide a component method, cancelDelete, which
          • sets showDeleteDialog back to false and logIndexToDelete to null

Main Vuex Store

  • Provide a setAllLogsToReady mutation, or equivalent, which iterates through all logs and sets each of their isReadytoSync property to true. (NB: this could also be done by providing a generic updateLogs() action, separate from the existing updateCurrentLog() action)
  • Provide a deleteLog action, which takes a log index as payload, and deletes it from the logs array on the state tree

Data Plugin

  • Subscribe to the setAllLogsToReady action, and when it's observed...
    • Filter all logs, where isReadytoSync && !isSynced === true
    • Dispatch pushToServer action, with all filtered logs as payload
  • Subscribe to the deleteLog action, and when it's observed...
    • Dispatch deleteRecord action, passing in the local_id of the payload

DB Module

  • Provide a deleteRecord action, which takes a local_id as payload and deletes it from the WebSQL table

HTTP Module

  • Provide a pushToServer action, which
    • Checks credentials
      • If credentials are not stored, push a reroute to /login
      • If credentials are stored, attempt first sync
    • Iterates through the payload. For each log:
      • Send log via AJAX to server
        • If successful, update the log's state with following response info:
          • Set sync property to 'synced'
          • Timestamp when log was synced
          • Remote log ID (from Drupal)
          • Log's remote URL
        • If unsuccessful because of no connection:
          • Set connection status to "Offline"
          • Bubble up error to user so they know to try again later
          • Schedule a retry based on some time limit, with exponential back-off
        • If unsuccessful because of 403 error
          • Push a reroute to /login
          • Schedule a retry based on successful login event?

Notable changes

Apart from changes to make this more idiomatic to Vue, I've departed from Mike's original model in 2 significant ways...

Optimistic Network Requests

Instead of testing the connection first, I want to take the optimistic approach and assume a viable connection, and fallback on the offline behavior only after the first attempt fails, as an error handling strategy essentially. This has a several advantages:

  • The API for testing the network, navigator.connection, is notoriously unreliable, since it can only test the first hop in an HTTP request, which may only be to a local router, and often gives a false positive if the network is unavailable beyond that first hop.
  • Because navigator.connection is so unreliable, we need to have proper error handling regardless, so we can schedule a retry, or propagate that error to the user somehow; with that error handler in place, preemptively testing the network becomes an extra step we no longer need.
  • With multiple observations, and multiple requests, some of which might include large binary data files, it is quite reasonable to expect that only a portion of the total requests could fail, in which case it is more useful to have a proper error-handling strategy that will handle each request individually.

Deleting partial log data

I'm not sure why we want to delete specific log properties, if we're not ready to delete the whole log. This seems like adding unnecessary complexity; currently, the logs are stored in the Vuex store as an array of log objects of uniform type (ie, all the same properties), so we'd just be setting those properties to null or "", which doesn't seem to gain us much in the way of memory and seems like it could introduce some unforseen errors. We also have to make sure these deletions are propagated to local persistence. As of now, we actually have no delete methods for the local db, and I'm going to be wading in to the WebSQL API very slowly until I get more familiar with it and Alex's schema.

The simpler option, and also the more pressing concern, as I see it, would be adding a policy for deleting synced logs after some period, once they're no longer needed.

If the concern is that stale properties could conflict with the server, I think it's easy enough to just make logs read-only after they've been synced, and only push logs to the server that are unsynced. And as long as that's the case, we can be more aggressive about purging those logs entirely and freeing that memory, since they're basically just "receipts" of the server transaction, and can't really be used for much beyond linking to the remote farmOS site.

Later when we want to work out a consistency model with the server, I think we'll be at a better starting point if all the logs in the store have preserved all the properties they had at the time they were synced. I'm thinking specifically if we want to use a hashing algorithm to diff those logs against the server, but I think generally it will be simplest to if we can trust that each log is uniform and represents the most up-to-date data that's available locally.

@jgaehring
Copy link
Member

I've been thinking more about the sync state (ie, unsynced, pending and synced) and how best to represent that state and propagate changes when the 'Sync' button is pressed. First of all, lumping that idea of pending in with synced/unsynced, which should be a proper boolean property, might be a little greedy. I think it's actually a composite of 2 unique pieces of state: isReadytoSync and isSynced. That distinction is important b/c the former should be determined by user interaction, while the latter is determined by the success or failure of the network request. The client should then render the spinner if isReadytoSync && !isSynced, which is a basically equivalent to pending, just more explicit.

By enforcing this separation between UI state and network state, we also get the advantage of using Vuex's built-in observables again, as we've been using for all other data plugin actions; that is, we can add another subscribe method to the data plugin, which listens for changes to the ready state, and then dispatches its own pushToServer action. That way the client knows even fewer details about how and when the data plugin does its syncing, it's just passing on the user's intent.

@mstenta
Copy link
Member Author

mstenta commented May 14, 2018

All great thoughts and details! +1

mstenta referenced this issue in farmOS-legacy/farmOS.org Jun 1, 2018
I originally posted this in a comment to [Issue #3](farmOS/field-kit#3 (comment)). It should still be cleaned up and expanded, but I feel it hits all the major beats of how we're leveraging the Vue ecosystem for the client and data plugin. The login plugin should be addressed, too, but it's pretty similar to the data plugin; it's also in need of some overhaul so I'd like to wait til that's done.
mstenta referenced this issue in farmOS-legacy/farmOS.org Jun 1, 2018
I originally posted this in a comment to [Issue #3](farmOS/field-kit#3 (comment)). It should still be cleaned up and expanded, but I feel it hits all the major beats of how we're leveraging the Vue ecosystem for the client and data plugin. The login plugin should be addressed, too, but it's pretty similar to the data plugin; it's also in need of some overhaul so I'd like to wait til that's done.
@jgaehring
Copy link
Member

AllObservations.vue has been complete for a while now. Closing...

@mstenta mstenta transferred this issue from farmOS-legacy/farmOS-client Feb 19, 2019
@mstenta
Copy link
Member Author

mstenta commented Feb 19, 2019

(Transferring all issues from old repository. See #92)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants