Solves traceback as per issue #445

Solves traceback as per issue #445

[RogerHaase, 2014-07-04 14:32:50]

looks ok, wait for tw review

[Thomas.J.Waldmann, 2014-07-05 13:13:12]

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.py
File MoinMoin/util/notifications.py (right):

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.p...
MoinMoin/util/notifications.py:101: if self.revs:
maybe to be more consistent with code below, use len(self.revs) > 0 here.

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.p...
MoinMoin/util/notifications.py:111: abort(403)
what precisely is forbidden here?

[ajgupta93, 2014-07-07 13:05:11]

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.py
File MoinMoin/util/notifications.py (right):

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.p...
MoinMoin/util/notifications.py:101: if self.revs:
On 2014/07/05 13:13:12, Thomas.J.Waldmann wrote:
> maybe to be more consistent with code below, use len(self.revs) > 0 here.

Ok.. I'll do that

https://codereview.appspot.com/109410043/diff/1/MoinMoin/util/notifications.p...
MoinMoin/util/notifications.py:111: abort(403)
On 2014/07/05 13:13:12, Thomas.J.Waldmann wrote:
> what precisely is forbidden here?

Basically this object gets it revs value from the function
"get_item_last_revisions". This searcher in this function fails to find any
revisions in case the user does not have permission to access the item specified
and hence returns empty list. This in turn causes an error here. So this error
happens when one changes the acl such that they do not have permission to read
the item in the new acl. So thats why abort(403).

[Thomas.J.Waldmann, 2014-07-12 15:20:59]

https://codereview.appspot.com/109410043/diff/60001/MoinMoin/util/notificatio...
File MoinMoin/util/notifications.py (right):

https://codereview.appspot.com/109410043/diff/60001/MoinMoin/util/notificatio...
MoinMoin/util/notifications.py:102: # get_item_last_revisions() returns an empty
list to self.revs
hmm, this is bad somehow.

it should raise some AccessDenied or so exception.

and item could theoretically really have no revisions and we need to
differentiate that case from permission denied. please add a TODO: to fix this
later.

[ajgupta93, 2014-07-14 13:05:55]

https://codereview.appspot.com/109410043/diff/60001/MoinMoin/util/notificatio...
File MoinMoin/util/notifications.py (right):

https://codereview.appspot.com/109410043/diff/60001/MoinMoin/util/notificatio...
MoinMoin/util/notifications.py:102: # get_item_last_revisions() returns an empty
list to self.revs
On 2014/07/12 15:20:59, Thomas.J.Waldmann wrote:
> hmm, this is bad somehow.
> 
> it should raise some AccessDenied or so exception.
> 
> and item could theoretically really have no revisions and we need to
> differentiate that case from permission denied. please add a TODO: to fix this
> later.

Done.

[Thomas.J.Waldmann, 2014-07-17 16:17:19]

https://codereview.appspot.com/109410043/diff/80001/MoinMoin/util/notificatio...
File MoinMoin/util/notifications.py (right):

https://codereview.appspot.com/109410043/diff/80001/MoinMoin/util/notificatio...
MoinMoin/util/notifications.py:189: # and to also to handle the case where the
item has no revisions
don't put comments above docstrings.
usually the docstring is the first thing after the header.