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

Updates not visible in items before syncing #86

Closed
tomaszn opened this issue May 4, 2017 · 13 comments
Closed

Updates not visible in items before syncing #86

tomaszn opened this issue May 4, 2017 · 13 comments

Comments

@tomaszn
Copy link
Contributor

tomaszn commented May 4, 2017

An updated item is "cloned to the outbox", with the original one's data not updated. This poses a problem, because I'd like to edit and display different objects while offline.

Are there any plans to support this use case? Any ideas for a solution or a workaround would also be very valuable. I'd also like to be able to update items several times before syncing.

I thought about patching model.find to overwrite fields from a corresponding item (by id) if it exists in unsyncedItems, but it may slow down the API significantly. Maybe just updating the local item when creating one is created in the outbox?

@sheppard
Copy link
Member

sheppard commented May 4, 2017

Yes, there are some rough edges here. The outbox API is structured around the idea of form submissions that may or may not have an associated model (the outbox actually predates the model API). The underlying principle is that the local model should not be updated until the item is synced with the server, to ensure you know whether you need to run a sync again (see https://wq.io/docs/outbox-js).

The downside, of course, is that it is tricky to do a lot of work offline with interrelated editable models. In the long term, I could see switching the logic around so that the model API always has your latest local version, with a flag to indicate whether a particular record has been synced or not.

In the short term, it is possible to workaround these issues with some tricks. Displaying and editing and outbox mostly work already - as long as you only use the outbox URLs (/outbox/1 and /outbox/1/edit) and not the model URL. However, both the unedited and edited records show up in list views by default. If you were only dealing with a single model, you could just remove the unedited record from the model with a postsave hack:

// Hack to override postsave/presync activity (should really have a plugin hook for this)
var _appPostsave = app.postsave;

app.postsave = function(item, backgroundSync) {
    var id = getId(item);
    if (id && backgroundSync) {
        return app.models[item.options.modelConf.name].remove(id).then(function() {
            return _appPostsave(item, backgroundSync);
        });
    } else {
        return _appPostsave(item, backgroundSync);
    }
};

getId() would look something like this:

function getId(item) {
    // id is usually only detectable from the URL (it's not a form field)
    // c.f. _renderOutboxItem
    var idMatch = item.options.url.match(
        new RegExp(item.options.modelConf.url + '/([^\/]+)$')
    );
    if (idMatch) {
        return idMatch[1];
    }
}

Unfortunately, this will temporarily break any foreign key links from other related models. One workaround for this would be to set up a context plugin that would detect this situation:

define(['wq/outbox'], function(outbox) {

return {
    'context': function(ctx, routeInfo) {
        if (routeInfo.page != 'childrecord') {
            return;
        }
        var context = {};
        outbox.model.load().then(function(items) {
            items.list.forEach(function(item) {
                 if (item.options.modelConf.name == 'parentrecord') {
                      if (getId(item) == ctx.parentrecord_id) {
                          context['parentrecord_outbox_id'] = item.id;
                      }
                 }
            });
        })
    }
};

});

Then you would need to detect this in the template:

<!-- childrecord_detail.html -->
<html>
  <h1>{{label}}</h1>
  {{#parentrecord_outbox_id}}
  <a href="{{rt}}/outbox/{{parentrecord_outbox_id}}">{{parentrecord_label}}</a>
  {{/parentrecord_outbox_id}}
  {{^parentrecord_outbox_id}}
  <a href="{{rt}}/parentrecords/{{parentrecord_id}}">{{parentrecord_label}}</a>
  {{/parentrecord_outbox_id}}
</html>

@tomaszn
Copy link
Contributor Author

tomaszn commented May 5, 2017

Thanks a lot!
One bit missing for now would be implementation of routing {{rt}}/parentrecords/{{parentrecord_id}}/childrecords, from where I need to access modified fields of parentrecord.
A solution would probably solve also #77 (running plugins for outbox items) and CSS styling if someone uses [data-url^="parentrecords/"] selectors in rules.

@sheppard
Copy link
Member

sheppard commented May 5, 2017

Ok... so you could do the same thing I did in 68efcba, which is essentially to make more outbox/ URLs map to their non-outbox equivalents:

router.register('outbox/<slug>/childrecords', function() {
// generate context and render page
});
router.addRoute('outbox/<slug>/childrecords', 's', function() {
// run plugins
});

But, I'm starting to be convinced that having all of these duplicate routes is unecessary. I would be willing to support a boolean configuration option, off by default (for now), that will automatically update the local models as soon as the form is saved. Once you turn it on, the outbox would then only be for showing sync status, and you wouldn't need to access any of the /outbox/* URLs. (You could even build apps that don't use the outbox or a server at all, instead managing and updating all data locally).

For now, you can emulate this by updating instead of deleting the unedited record:

// Hack to override postsave/presync activity (should really have a plugin hook for this)
var _appPostsave = app.postsave;
app.postsave = function(item, backgroundSync) {
    if (backgroundSync) {
        // FIXME: handle JSON forms (see app.js)
        // _parseJsonForm(item)

        var newRecord = item.data;
        newRecord.id = getId(item) || 'outbox-' + item.id;

        // FIXME: avoid having two outbox records for the same record
        // (be sure to erase after sync)
        // newRecord.outbox_id = item.id

        return app.models[item.options.modelConf.name].update([newRecord]).then(function() {
            return _appPostsave(item, backgroundSync);
        });
    } else {
        return _appPostsave(item, backgroundSync);
    }
};

You could then use the stock templates and routes mostly as-is, except that you'd might want to disable or modify the links in the {{#unsyncedItems}} section of the list template.

Note: This approach assigns a temporary id ('outbox-' + item.id) which will need to be updated after a sync. Since [model].update() does not handle ID changes, the synced record will be appended to the model rather than merged with the outbox record. So, you'll probably need to add some code to clean up after syncing. You probably don't need to handle parentrecord_id on childrecords since that will be automatically updated after the sync.

@tomaszn
Copy link
Contributor Author

tomaszn commented May 5, 2017

Thanks!
In a scenario when the application is closed when offline and restarted when online these new/updated items disappear. I tried copying them into context.list by a context plugin "used" before other plugins, but I think the context plugins are not executed in a chain to allow modifying the list.
Is there a hooking mechanism (postload?) I could use to copy unsynced items back to the list?

@sheppard
Copy link
Member

sheppard commented May 5, 2017

There's no official hook but you could do it after prefetchAll() in main.js:

app.prefetchAll().then(recopyOutboxItems);

You could also just remove the default call to prefetchAll() and have the user choose when to sync.

@tomaszn
Copy link
Contributor Author

tomaszn commented May 12, 2017

Thank you. With these steps I managed to move forward several steps, but I think new items don't get properly copied and I need workarounds in templates.
Is there a target date for a preliminary support for the unified-local-storage switch? I'll be glad to test it and report any issues.

@tomaszn
Copy link
Contributor Author

tomaszn commented May 29, 2017

Any chance that you looked at these modifications? If not, I can try to start working on them.

@sheppard
Copy link
Member

Hi, I haven't had a chance to look at this yet. If I implement it myself, it will probably be some time after the 1.0 release. If you can work on it, I can certainly review and hopefully merge your changes before then.

@tomaszn
Copy link
Contributor Author

tomaszn commented Oct 14, 2017

I found a universal offline storage library that (among other features) allows overlaying pending changes as "offline effects". It can also use localForage as its backend. If integrated with the wq models API it would allow easy customizations for each of the models.
https://github.com/redux-offline-team/redux-offline

@tomaszn
Copy link
Contributor Author

tomaszn commented Oct 24, 2017

I didn't manage to implement these updates, but I'm steadily interested in resolving this issue.
If by any chance you decided to go for it, please share a list of subtasks, so I can actively help in smaller steps.

@sheppard
Copy link
Member

sheppard commented Apr 5, 2018

I finally had a chance to look at redux-offline and agree it could address a lot of syncing issues (or at least make them easier to reason about). See #105 for more thoughts.

@tomaszn
Copy link
Contributor Author

tomaszn commented Oct 9, 2018

I just noticed another project that could solve these issues:
https://github.com/oracle/offline-persistence-toolkit

It supports "shredders", which can be used to filter results while offline.

@sheppard
Copy link
Member

I was finally able to integrate redux-offline for the outbox (see #113). From there, it was mostly a matter of supporting multiple strategies for dispatching model state updates (6580895). In the default (backwards-compatible) mode, a SUBMIT action is dispatched first, with UPDATE only dispatched after the sync is successful. In the new "IMMEDIATE" mode, UPDATE is dispatched immediately and followed up with a SUCCESS action on sync. I also added some code to ensure models are updated with IDs from the server (2b75917).

I updated the outbox documentation here: https://wq.io/1.2/docs/outbox-js

I imagine there may be a few edge cases to sort out, but this seems to be working pretty well overall. Let me know if you have any thoughts on the API. I am hoping to address most of the outbox issues for the wq.app 1.2 release.

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

No branches or pull requests

2 participants