Web Client: Shared Copy Bucket Owner No Longer Displays

Bug #1537223 reported by Jennifer Pringle
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned
2.10
Won't Fix
Undecided
Unassigned
2.11
Won't Fix
Undecided
Unassigned

Bug Description

Web Client

When a copy bucket is shared with another user the system no longer displays the username of the owner of the bucket.

Having the owner name display is a nice visual queue that you are looking at a shared bucket and not one of your own.

Kyle Huckins (khuckins)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Revision history for this message
Kyle Huckins (khuckins) wrote :

Pushed a fix up here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=79358b9e7e0d7718ba960fda65b5bf9d16c12562

In anticipation of a planned fix for https://bugs.launchpad.net/evergreen/+bug/1527770, I've made use of spans using ng-bind, rather than {{ }}.

One thing to note, and something I ran into while developing this branch, the username will display only if you can view users on the level of that users' home library(ex. Staff with a view user permission level of System will not be able to see the username of a user whose home library is consortium). This is a permissions issue, and to get around it may end up needing either an adjustment to the fieldmapper to get n owners username into the ccb class. Currently we're getting the buckets' owner(the id of an au), and can't get any deeper, so we have to retrieve the specific user, then grab their username.

tags: added: pullrequest
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Kyle. Some feedback..

1. See discussion re: ng-bind in bug #1528916

2. On the topic of bucket display issues, see also bug #1541060. Work done here (copy buckets) will need to be done for the other (record buckets) and vice versa. Make the bugs dupes?

3. This may be finicky, but here goes.. when hanging extra data off the 'bucket' object (bucket.owner_name), which is an IDL object with a well-defined structure, I suggest adding an underscore (bucket._owner_name) to make it obvious to the code reader this is a throwaway/temporary value that is not part of the IDL object proper. Alternatively, you could go the bucket.owner().usrname() route, but beware it could affect how other code accesses the owner() data on the bucket.

4. Regarding permissions, I'm fairly certain that having global VIEW_USER permissions is standard Evergreen practice and may actually be a requirement for basic staff client usage.

Revision history for this message
Mike Rylander (mrylander) wrote :

If we ever actually create a code style guide, the point in (3) should be there enshrined for all eternity!

Regarding (4), there are /perhaps/ cases where a staff member may not have global VIEW_USER permission, such as where org hiding is in use, but there are so many other rough edges to worry about in that case than this specific point that it's not worth considering as a practical concern, and might not even be visible in reality simply because of workflow. Which is to say, I agree that for the purpose of fixing this bug we should assume global VIEW_USER permission.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Thanks for the feedback!

2. I'll take a look at that one, and make sure it gets the proper treatment.

3. That makes sense. I initially tried the owner().usrname() route, however it consistently returned null, as did any other attempts to go further past owner(). The underscore should be a quick and easy change.

4. That's good to know, there shouldn't be any issues that arise from permissions then.

Revision history for this message
Kyle Huckins (khuckins) wrote :
Revision history for this message
Bill Erickson (berick) wrote :

Hi Kyle, it looks like the ng-show="..." also needs updating to reflect the change to _owner_name.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Oops, thanks for the catch! I've amended the commit with the fix.

Revision history for this message
Bill Erickson (berick) wrote :

I have marked bug #1541060 as a duplicate. To round out the code, we'll need to owner, owner's home library, and the bucket ID # to display in both the copy and record bucket interfaces.

Revision history for this message
Kyle Huckins (khuckins) wrote :

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1537223-buckets-username-display

I've moved the work to a new branch with a better reflecting name, and made the required additions to handle record buckets as well. The changes needed were exactly the same, which made things easy, but it also makes me wonder if it might be good to look at the need to have the copy and record bucket templates separated. t_bucket_info.tt2, for example, is exactly the same between both versions.

Revision history for this message
Bill Erickson (berick) wrote :

Yes, let's consolidate like files. By my reckoning, these can all be shared:

t_bucket_create.tt2
t_bucket_delete.tt2
t_bucket_edit.tt2
t_bucket_info.tt2
t_bucket_selector.tt2
t_load_shared.tt2

Consistent with other shared files, I suggest moving these files to a new directory:

Open-ILS/src/templates/staff/cat/bucket/share/

Template and JS references to these files will of course have to be updated.

Once moved over, all changes for this bug can be applied to share/t_bucket_info.tt2

===

When we eventually implement call number and user buckets, this will also make the task much quicker.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've applied the changes and pushed them up on the same branch.

Revision history for this message
Bill Erickson (berick) wrote :

Great, I believe all that's left then to satisfy bug #1541060 is to display the home library shortname [ egCore.org.get(patron.home_ou()).shortname() ] of the owner and bucket ID in the summary as well.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Oops, got caught up in the merging that I forgot about the ID and OU, made the changes, referencing the existing staff client for placement, and pushed them up.

Bill Erickson (berick)
Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
status: New → Confirmed
milestone: none → 2.11.1
Revision history for this message
Kathy Lussier (klussier) wrote :

Thank you Kyle! There was some talk above about displaying the owner name, but one thing I see in the new code that differs from our existing record bucket display is that, instead of displaying the owner's last name followed by their patron barcode in parentheses, we now just see the user name. Side-by-side screenshot at http://www.screencast.com/t/74jg2yCKtS7T.

I don't have a strong opinion on whether the new display is preferred or not, but I wanted to point out the difference to see if others have an opinion. If the owner name is just a visual cue that the bucket is shared, as suggested in Jennifer's original report, this may be fine. But if there are reasons staff need to identify who the owner is or pull up that person's record (most easily done by barcode) we may want to consider displaying those other elements.

Any thoughts out there?

Changed in evergreen:
milestone: 2.11.1 → 2.next
Revision history for this message
Bill Erickson (berick) wrote :

Pushed a sign-off for Kyle's changes.

Pushed a secondary commit that makes the owner info display consistent with the XUL client. This includes displaying the last name followed by the barcode in parens. (If barcode is null, the username is displayed). The barcode is also now rendered as a link to the patron display for the bucket owner.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1537223-webstaff-bucket-owner

Changed in evergreen:
milestone: 2.next → 2.12-rc
Revision history for this message
Dawn Dale (ddale) wrote :

"I have tested this code and consent to signing off on it with my email address, [<email address hidden>], and name, [Dawn Dale]".

tags: added: signedoff
Revision history for this message
Kathy Lussier (klussier) wrote :

Thank you Kyle, Bill and Dawn! Merged to master for inclusion in the 2.12 RC.

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.