User talk:OliverCharles/NGS Edit Stash: Difference between revisions

From MusicBrainz Wiki
Jump to navigationJump to search
No edit summary
(New section: My comments)
Line 6: Line 6:


Other than those comments this looks good to me! --[[User:RobertKaye|RobertKaye]] 08:20, 3 August 2009 (UTC)
Other than those comments this looks good to me! --[[User:RobertKaye|RobertKaye]] 08:20, 3 August 2009 (UTC)

== My comments ==

There are basically two possible approaches, either we store a serie of actions or values and I think ruaok's idea was to store actions rather than just current value. The difference is that if you store actions, you have:

<pre><nowiki>
$data = [
{ type => 'edit', name => 'track.0.name', value => 'F' },
{ type => 'edit', name => 'track.1.name', value => 'Bar' },
{ type => 'edit', name => 'track.0.name', value => 'Foo' },
];
</nowiki></pre>

and if you store just values you have:

<pre><nowiki>
$data = {
'track.0.name' => 'Foo',
'track.1.name' => 'Bar',
};
</nowiki></pre>

I think it doesn't matter which one we will choose, they are both "good enough". List of actions is more complex to handle, but allows us to do merging and implement undo/redo. Hash of values is simpler to handle, but we can't resolve conflicts or do any other advanced things.

Regarding the client side, I'd do it a little differently. Either watch for 'change' events or have the use click per-field "Save" (like Review Board). But instead of sending the data at the end of the editing session, send them shortly after they are changed. For example, user changes a track name, have a 10 second timeout, if nothing else happens during that time, send the data to the server (only the change, not the whole stash). If the user does change something else, restart the timer and try again. I think this is safer than waiting for the user to navigate out of the page and with properly chosen timeout it shouldn't cause that many requests.

(Btw, I'd call this feature Draft, not Stash.)

:: ''The server stores the HTTP referer variable as to where to restore the stash to. ''

I wouldn't trust the referer header, we should explicitly say what do we want to save and where.

:: ''stash XML or TEXT?,''

My vote is for XML, so that we can query it if we need it (upgrade scripts, for example).

:: ''edit_url VARCHAR''

I'd prefer to split this into 'entity_type', 'entity_id' and possibly 'form'. This way we can easily have a bar on the main release page saying "Hey, you have 2 saved drafts for this release. Do you want to edit them?".

[[User:LukasLalinsky|Lukáš Lalinský]] 21:32, 3 August 2009 (UTC)

Revision as of 21:32, 3 August 2009

Comments:

  • "If a stash already exists, it merges changes into this stash, always using the newer version in any merge conflicts."
    • I think we should just overwrite the old stash for now. Just to keep it simple -- then we can work on merge. The same goes for the conflicts section of Open Issues.
  • "Do we ever expire stash entries. E.g. after a month of inactivity?"
    • Yes. But later. And even then someone will complain that the period isn't long enough. :)

Other than those comments this looks good to me! --RobertKaye 08:20, 3 August 2009 (UTC)

My comments

There are basically two possible approaches, either we store a serie of actions or values and I think ruaok's idea was to store actions rather than just current value. The difference is that if you store actions, you have:

$data = [
    { type => 'edit', name => 'track.0.name', value => 'F' },
    { type => 'edit', name => 'track.1.name', value => 'Bar' },
    { type => 'edit', name => 'track.0.name', value => 'Foo' },
];

and if you store just values you have:

$data = {
    'track.0.name' => 'Foo',
    'track.1.name' => 'Bar',
};

I think it doesn't matter which one we will choose, they are both "good enough". List of actions is more complex to handle, but allows us to do merging and implement undo/redo. Hash of values is simpler to handle, but we can't resolve conflicts or do any other advanced things.

Regarding the client side, I'd do it a little differently. Either watch for 'change' events or have the use click per-field "Save" (like Review Board). But instead of sending the data at the end of the editing session, send them shortly after they are changed. For example, user changes a track name, have a 10 second timeout, if nothing else happens during that time, send the data to the server (only the change, not the whole stash). If the user does change something else, restart the timer and try again. I think this is safer than waiting for the user to navigate out of the page and with properly chosen timeout it shouldn't cause that many requests.

(Btw, I'd call this feature Draft, not Stash.)

The server stores the HTTP referer variable as to where to restore the stash to.

I wouldn't trust the referer header, we should explicitly say what do we want to save and where.

stash XML or TEXT?,

My vote is for XML, so that we can query it if we need it (upgrade scripts, for example).

edit_url VARCHAR

I'd prefer to split this into 'entity_type', 'entity_id' and possibly 'form'. This way we can easily have a bar on the main release page saying "Hey, you have 2 saved drafts for this release. Do you want to edit them?".

Lukáš Lalinský 21:32, 3 August 2009 (UTC)