Failed "login" requests are stored in outbox

[tomaszn]

I think it's a bug, either in wq.app or wq.start, because:

1. a simple outbox syncing can switch user accounts
2. the password is kept in the outbox for indefinite time

Steps to reproduce:

1. click "login" on the main page
2. type incorrect username and password
3. click Login
4. run in console: require('wq/outbox').unsyncedItems().then(function(items){console.log(items[0].data)}) to see the form data

[sheppard]

Yes, this is a bug. This issue is primarily in wq.app's JS, though the solution might involve adding a data-attribute to the wq.start login.html template. The solution is either to:

1. add a "temporary" option for outbox items to ensure the actual values are not stored in the model cache. We'd probably still store a placeholder record in the outbox to avoid having to add a bunch of custom logic.
2. alternatively, just bypass the outbox entirely (by implementing a separate outbox-free AJAX routine). This can sort of be done by setting data-wq-json=false on the <form> element, though in this case we still need wq to handle the response and update app.user etc..

[tomaszn]

I rather see this as a special case of data-wq-background-sync="false" that provides "online-only" forms (for any model). Maybe a third value "never" could be used for that?

[sheppard]

Good point, this really should be tied to the background sync flag. After thinking about this some more, I think data-wq-background-sync="false" should be online-only in its default case. It's true that background-sync was originally designed only to control the user experience after form submission. But the use or non-use of the outbox is certainly related. Thinking about the four possible variations:

1. Sync in background, store in outbox (background-sync="true")
2. Sync in background, skip outbox (not supported)
3. Sync in foreground, store in outbox (current meaning of "false")
4. Sync in foreground, skip outbox (ideal meaning of "false")

Option 1 and 4 seem like they should be the main options. Option 2 and 3 seem less useful and I would probably wait to support them until someone needed them for a real use case.

[tomaszn]

There is another variation, which I currently need:
5. Don't sync at all, store in outbox ("store"?)

The whole storing, syncing, refreshing cycle can be unpredictably slow in weak network conditions and just storing form data for syncing later is a solution for that.

Additionally, I can't find a reliable way to test behaviour of the application in the offline state (SeleniumHQ/selenium@7ba13118ac doesn't work as expected, so I reported https://bugs.chromium.org/p/chromium/issues/detail?id=781921). So this variation would unify behaviour across different network conditions and allow testing the application with Selenium.

[sheppard]

I don't think it's documented, but in theory option 5 can be set at the application level by setting config.backgroundSync to a negative number.

config.backgroundSync = false;    // Always sync in foreground
config.noBackgroundSync = true;   // Always sync in foreground
config.backgroundSync = true;     // Background sync, retry every 30 seconds
config.backgroundSync = 60 * 5;   // Background sync, retry every 5 minutes
config.backgroundSync = -1;       // Sync only on demand (e.g. Sync Now button)

This should probably be tested (and thought through a bit more).

[tomaszn]

In my fork, when background sync is used, there seems to be a race condition if new records are constantly provided, which results in loss of entries before they are synced. I can reproduce it 100% of times when the form submission cycle is short (like accepting default data) and network connection is slowed down (in Chrome DevTools / Network). Could it be a problem in the way model.overwrite() is used in .update, .remove etc.?

When config.backgroundSync = false is set, records are lost if the submission fails.

So it looks like setting -1 is the safest option, provided that #102 is fixed.