Setting appropriate group in getEventLogsByPeriod context (see #9609) #370
Conversation
|
I don't know if you are getting an exception, etc. but I doubt that it can (ever?) be considered the wrong thing to do to pass an explicit group. |
|
@joshmoore: Looks like the exception @cneves is trying to work around is in #9609 (http://trac.openmicroscopy.org.uk/ome/ticket/9609). I wonder if this would be fixed or affected by your changes on #369? |
|
Certainly possible, though I haven't recently fixed any NPEs specifically. Might be worth trying to reproduce at all today with various combinations of develop, sprint5-bugs, and this branch. |
|
Bug fixed. Page no longer throws an error. Will write a unit test for |
Added duplicate tests to several of the methods in itimline.py. Two tests are still failing but were doing so before these changes. Also removed the duplicate test1175 which was identical to the other method of the same name.
|
@joshmoore, the offending case was based on having a non-admin user part of a group with events, and then running the timeline query with omero.group=-1. I believe the case tested had the user's event context group being something other than the group he was searching on, too. I can try to write a test that creates the appropriate graph for triggering this, if needed. |
|
@cneves, adding a test surely wouldn't hurt, but not holding up release for that. |
Setting appropriate group in getEventLogsByPeriod context (see #9609)
The issue I'm trying to fix is one more case of having omero.group=-1 in ctx not working.
In this particular case even without being sure of wether this can be considered a server bug or not I'm fairly sure it is safe to properly set the omero.group because the query itself is filtered by omero.group and thus it is not apparently possible that results would ever be hit outside the proper event context group.
@joshmoore: I might be speaking out of turn when I say this is not a server bug, can you review that please?