[Contrib: Evaluation System] Proposed changes to make hierarchy more useable

Thu Oct 27 16:02:31 PDT 2011

Thanks for your questions Aaron - I'm very happy to have some discussion
about this before committing anything.

Re: EVALSYS-967

Ah, I see, so originally, hardcoding in "admin" made sense, because it would
return all groups. But now it doesn't - it only returns the groups 'admin'
is explicitly enrolled in.

Currently, the code does
1) Get all evalgroups for admin
2) See if any of those groups are checked

Since #1 doesn't get all groups in the system (as Aaron indicates it once
did), I basically propose changing the order up a bit:
1) Get all checked groups for this node, regardless of if admin or the
current user has access to them. This clearly shows the current user what
groups are in this node, without any restrictions.
2) Look at the current courses the user is enrolled in, eliminate any
courses we've already listed in #1 and add the rest as unchecked. This
allows the user to add additional courses.

Ideally, instead of #2, we'd have an interface where the user could look up
any course in the system and add it, but that's a bit more work and I feel
this is a good interim measure.

If the only change we made was to change "admin" to the current user, then
we'd be making an assumption. We'd be assuming that someone has to have
"EvalConstants.PERM_BE_EVALUATED" in a course before we can allow them to
associate it to a point in the hierarchy or even see that this course is
associated.

Is that a valid assumption considering that this person is an evaluation
administrator? If they are an administrator with access to "Administrate"
and "Control Hierarchy" then why does this need to be enforced? I would
argue it only puts up an unnecessary barrier to them doing their job.

Unless there's another interface where this code is used besides in
Administrate > Control Hierarchy?

I've added this as a comment to the JIRA too.

Re: EVALSYS-1146

I wasn't aware of EvalCommonLogic.makeEvalGroupObject - I'll see if I can
use that instead.

Thanks,
Nicola

On Wed, Oct 26, 2011 at 6:29 AM, Aaron Zeckoski <azeckoski at unicon.net>wrote:

> There seems to be an unnecessary change in
> https://jira.sakaiproject.org/browse/EVALSYS-967
>
> The point of the locateBean method is to find all the groups the user
> should be able to deal with and then show them as checked if they were
> already assigned or unchecked if they were not. Since the method
> getEvalGroupsForUser was changed awhile back to no longer return all
> groups when the user is an admin (I forget who made this change), the
> code doesn't quite work the way it used to. This is why this change to
> line 64 is now necessary.
> However, for some reason, the checked/unchecked filtering was changed
> as well and I don't see why that part of the change is necessary.
> Maybe some extra explanation in the ticket would help.
>
>
> https://jira.sakaiproject.org/browse/EVALSYS-1146
> does not follow best practices and uses a sakai interface directly
> (SiteService) instead of using EvalCommonLogic.makeEvalGroupObject but
> in the case of the UI it is probably tolerable though I would prefer
> it not be the case. Otherwise it looks like it is probably OK (though
> a bit slow for large numbers of groups).
>
>
> -AZ
>
>
>
> On Tue, Oct 25, 2011 at 4:09 PM, Nicola Monat-Jacobs
> <nicola at longsight.com> wrote:
> > Thanks Aaron. It looks like EVALSYS-1146 did actually need a patch also.
> If
> > you have a sec to review it, that would be awesome.
> > Thanks!
> >
> > On Mon, Oct 24, 2011 at 5:03 PM, Aaron Zeckoski <azeckoski at unicon.net>
> > wrote:
> >>
> >> Sounds good to me.
> >> It does circumvent some possible security but I think that is probably
> OK.
> >> -AZ
> >>
> >>
> >> On Mon, Oct 24, 2011 at 7:45 PM, Nicola Monat-Jacobs
> >> <nicola at longsight.com> wrote:
> >> > There are two outstanding issues with the hierarchy interface that I
> >> > want to
> >> > run by everyone.
> >> > https://jira.sakaiproject.org/browse/EVALSYS-967
> >> > This has been an annoyance for a while - it means that you can only
> >> > assign a
> >> > course site (or 'evalgroup' as it is in know in evalsys) to a position
> >> > in
> >> > the hierarchy if the 'admin' user is enrolled in that course site.
> This
> >> > means that admin has to be enrolled in all your course sites if you
> want
> >> > to
> >> > use hierarchy.
> >> > https://jira.sakaiproject.org/browse/EVALSYS-1146
> >> > Additionally, if you are successful in assigning your nodes through
> some
> >> > other means, those assignments won't always show in the GUI to the
> >> > current
> >> > user (confusing! user wants to be sure eval will go to the courses
> they
> >> > think it will). Even if we just swapped admin for currentUser (to
> >> > address
> >> > EVALSYS-967 above) the assigned courses won't show if the current user
> >> > isn't
> >> > enrolled in them.
> >> > So, the proposed solution for EVALSYS-967 addresses both these
> >> > solutions.
> >> > Instead of looking up all the sites 'admin' is enrolled in, and then
> >> > finding
> >> > out which of those are or are not assigned to this node, we go
> >> > a conceptually simpler route.
> >> > The proposed patch looks up and shows the sites currently assigned to
> >> > this
> >> > node (regardless of if the current user, or admin, is enrolled in
> them).
> >> > Then, it looks up and shows the courses that the current user (not
> >> > admin) is
> >> > enrolled in and offers those up to be added to the current node.
> >> > A more robust solution would allow the user to have some sort of
> lookup
> >> > interface that would allow them to search for any course in the
> system,
> >> > but
> >> > I think this is a good interim measure to allow folks to actually use
> >> > this
> >> > feature.
> >> > Thoughts?
> >> > - Nicola
> >> > _______________________________________________
> >> > evaluation mailing list
> >> > evaluation at collab.sakaiproject.org
> >> > http://collab.sakaiproject.org/mailman/listinfo/evaluation
> >> >
> >> > TO UNSUBSCRIBE: send email to
> >> > evaluation-unsubscribe at collab.sakaiproject.org
> >> > with a subject of "unsubscribe"
> >> >
> >>
> >>
> >>
> >> --
> >> Aaron Zeckoski - Software Architect - http://tinyurl.com/azprofile
> >
> >
>
>
>
> --
> Aaron Zeckoski - Software Architect - http://tinyurl.com/azprofile
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://collab.sakaiproject.org/pipermail/evaluation/attachments/20111027/71313e42/attachment.html 