Support for multiple names in Revert view

Support for multiple names in Revert view
Plus we should have a validation function here to check whether the old names (names that the previous rev had), have not been taken by some other item.
eg
create a item (item1) with name [mommy, daddy] (rev0)
rename it to [mommy] (rev1)
create another item (item2) with name [daddy].
XXX cannot revert rev1 back to rev2 as 'daddy' is already taken by item2.

[ashu1461, 2013-07-10 12:04:56]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/templates/revert.html
File MoinMoin/templates/revert.html (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/templates/revert.html...
MoinMoin/templates/revert.html:4: <h1>{{ _("Revert '%(item_name)s' (rev
%(rev_id)s, names '%(new_names)s') ", item_name=fqname | shorten_item_name,
rev_id=rev_id | shorten_id, new_names=item.meta['name'] | join('|')) }}</h1>
Showing the names of the old rev would make it more clear in case there is an
error.

[Thomas.J.Waldmann, 2013-07-10 14:57:39]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:521: invalid_name_msg = ''
?

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:528: names = set(prev_meta[NAME]) -
set(current_meta[NAME])
you don't need to do that (it might be a small optimization, but rather make
sure all the names we want are valid).

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:529: prev_meta[NAME] = list(names)
you are killing some of the names from prev_meta[NAME] here - why?

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:571: current_rev =
flaskg.storage.document(**item.fqname.query)
fqname.query???

[ashu1461, 2013-07-10 18:40:55]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:521: invalid_name_msg = ''
On 2013/07/10 14:57:40, Thomas.J.Waldmann wrote:
> ?
We later fill this up.

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:528: names = set(prev_meta[NAME]) -
set(current_meta[NAME])
On 2013/07/10 14:57:40, Thomas.J.Waldmann wrote:
> you don't need to do that (it might be a small optimization, but rather make
> sure all the names we want are valid).

Well this is necessary, How the current name validator works is it searches if
the names are present somewhere else (except the same item).

In case the previous revisions and the current revisions are of different items,
the name validator will catch the current revision also since the itemid is
different.

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:529: prev_meta[NAME] = list(names)
On 2013/07/10 14:57:40, Thomas.J.Waldmann wrote:
> you are killing some of the names from prev_meta[NAME] here - why?
see above.

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:571: current_rev =
flaskg.storage.document(**item.fqname.query)
On 2013/07/10 14:57:40, Thomas.J.Waldmann wrote:
> fqname.query???

A new method that returns the related query.

[Thomas.J.Waldmann, 2013-07-11 23:35:44]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:528: names = set(prev_meta[NAME]) -
set(current_meta[NAME])
> In case the previous revisions and the current revisions are of different 
> items,

How can that be?

> the name validator will catch the current revision also since the itemid is
> different.

Usually the itemid is created once for a new item and then always kept the same
in later revisions of same item.

[ashu1461, 2013-07-12 18:56:37]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:528: names = set(prev_meta[NAME]) -
set(current_meta[NAME])
On 2013/07/11 23:35:44, Thomas.J.Waldmann wrote:
> > In case the previous revisions and the current revisions are of different 
> > items,
> 
> How can that be?
> 
> > the name validator will catch the current revision also since the itemid is
> > different.
> 
> Usually the itemid is created once for a new item and then always kept the
same
> in later revisions of same item.

Usually, but there can be special cases.
Item1 = names = ['foo', 'bar', 'buz'] and rev = rev0
delete 'buz'
make a new item 
Item2 = names = ['buz', 'superbuz'] , rev = rev1

If we revert back to rev0, the itemid of earlier 'buz' is different from the
current 'buz' .. 
So i kill the names that appeared in the previous meta.

[Thomas.J.Waldmann, 2013-07-13 15:54:44]

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/1/MoinMoin/apps/frontend/views.p...
MoinMoin/apps/frontend/views.py:528: names = set(prev_meta[NAME]) -
set(current_meta[NAME])
i don't think we should support reverting based on the "history of a name",
but only by "history of a single specific itemid".

the itemid is identifying some specific item and will never change (and it
somehow links together all revisions of that item).
if you have different itemids, you have different items and we should not do
"cross-item reverting", just because someone chose the same name.

that said, showing history based on a name might be still interesting (but do
not offer revert there). we also need to show history based on itemid (and
should offer revert there).

[Thomas.J.Waldmann, 2013-07-13 23:52:42]

https://codereview.appspot.com/10973043/diff/11001/MoinMoin/apps/frontend/vie...
File MoinMoin/apps/frontend/views.py (right):

https://codereview.appspot.com/10973043/diff/11001/MoinMoin/apps/frontend/vie...
MoinMoin/apps/frontend/views.py:532: return True
you could move this upwards into the try block to make it easier.

https://codereview.appspot.com/10973043/diff/11001/MoinMoin/apps/frontend/vie...
MoinMoin/apps/frontend/views.py:568: state = dict(fqname=item.fqname,
itemid=item.meta.get(ITEMID), meta=dict(item.meta))
if you access it via ITEMID (see 528), you need to use ITEMID here also, as a
constant, not as hardcoded name.