victorjonsson / Arlima

Article List Manager - Wordpress plugin suitable for online newspapers that's in need of a fully customizable front page
28 stars 16 forks source link

Some fixes for future lists #76

Closed aaslun closed 9 years ago

aaslun commented 9 years ago

Here's my attempt to address the issues you pointed out @victorjonsson in https://github.com/aaslun/Arlima/commit/1d1a6333ef178f86b0692c667e0e18c4abea5c5b

aaslun commented 9 years ago

The functionality for displaying the reload notice is not as advanced as the proposed one. It simply appends the count-down notice in the list header at a given time limit. The time limit is configureable and defaults to 60 seconds. There was a request from our users to have the count down visible for a longer time and until the list reloads due to a notion that this could be missed otherwise. But of course, feel free to comment, tweak or adjust this.

victorjonsson commented 9 years ago

I have some input. Consider the following use case:

You have a bunch of editors working with Arlima during the day. Some of them only interacts with the list manager a couple of times during the entire day, others constantly makes changes to lists. All of these editors works with tabs in the browser. Reloading of the lists manager tab is seldom done. In fact, it might be open in days without reload for some of the editors.

The current solution does not support this scenario. The list manager is not a typical web page, you should think of it as a desktop client in some ways.

I will do some refactoring and try to support the above scenario

aaslun commented 9 years ago

Good points! I have some additional thoughts on that: I think this does not only apply to the scheduled lists use case. I may be overlooking something, but isn't there a current potential issue if more than one editor is working in the list manager at the same time as other editors and on editor saves changes to the lists? Lists may be overwritten without notice if multiple users are working at the same time as Arlima does not currently support global locking/unlocking check of lists when they change state to unsaved (similar to the way we used to do it in Incopolo using database flags). Is this something that we should take into account since this issue may relate to the scenario you describe?

A thought out of the blue on this: If the lists were hashed, could comparing hashes be a useful approach of detecting if changes been made to a list?

victorjonsson commented 9 years ago

The first thing that happens when someone tries to save a list is that the latest version is fetched from backend. If a newer version exists the author, wanting to save, has to confirm that the latest version should be overwritten.

But still, if two or more authors are working in parallell on the same list they could theoretically overwrite each others versions without notice. The odds of that happening is probably slim to null.

Having a locking state on lists in the user interface (as incopolo had) is not a good idea. It doesn't really solve any problem.. it only makes things frustrating for the author. If we're really scared of unintentional data overwrites we should implement the locking feature on a transaction level.

  1. User presses save
  2. The backend tries to lock the list object
  3. Backend acquires the lock
  4. The ui fetches the latest version and lets the user confirm if overwrite is about to happen
  5. Saves the new version
  6. Backend releases the lock
victorjonsson commented 9 years ago

Okey, now I think that future-lists-fix2 is ready for some extensive testing. I have tried to imagine all the possible scenarios where things could bork but I might have missed something of course...

aaslun commented 9 years ago

Nicely done! We'll start testing as soon as possible.