Summary: | Expose borrowernumber and branch when user is logged in to OPAC | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | OPAC | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart, martin.renvoize, pasi.kallinen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 20921: Add borrowernumber and branchcode as html attributes
Bug 20921: Add borrowernumber and branchcode as html attributes Bug 20921: Add borrowernumber and branchcode as html attributes Bug 20921: Add borrowernumber and branchcode as html attributes Bug 20921: Fix opac_auth for selenium Bug 20921: Add selenium tests for .loggedinusername |
Description
Nick Clemens (kidclamp)
2018-06-12 15:57:46 UTC
Created attachment 76005 [details] [review] Bug 20921: Add borrowernumber and branchcode as html attributes To test: 1 - Apply patch 2 - Visit the opac 3 - Sign in 4 - You should not see any difference 5 - Inspect the element 'Welcome, {username}' 6 - Note the attributes for borrowernumber and branchcode 7 - Add to OPACUserJS: $(document).ready(function(){ console.log( $(".loggedinusername").attr('branchcode')); console.log( $(".loggedinusername").attr('borrowernumber')); }); 8 - Check the console and note you can see the info expected I think we already have the branch in the source code, but in another spot. Custom HTML attributes should be prefixed with "data-" (as per the HTML5 spec) It's a class in the <body> tag: <body ID="advsearch" class="branch-CPL scrollto" > branch-CPL represents the branch of the currently logged in user. Maybe it would be cleaner to also add the borrowernumber there? Also the use of custom attributes appears strange to me - why use class for the username but then 2 attributes? (In reply to paxed from comment #3) > Custom HTML attributes should be prefixed with "data-" (as per the HTML5 > spec) I agree with this. I think it makes sense to add data- attributes for branch and borrowernumber even with the branch information in a class. Talked to Owen on IRC - all things cleared up. I think only thing missing here is adding the data- prefix. Created attachment 76200 [details] [review] Bug 20921: Add borrowernumber and branchcode as html attributes To test: 1 - Apply patch 2 - Visit the opac 3 - Sign in 4 - You should not see any difference 5 - Inspect the element 'Welcome, {username}' 6 - Note the attributes for borrowernumber and branchcode 7 - Add to OPACUserJS: $(document).ready(function(){ console.log( $(".loggedinusername").attr('data-branchcode')); console.log( $(".loggedinusername").attr('data-borrowernumber')); }); 8 - Check the console and note you can see the info expected Created attachment 76310 [details] [review] Bug 20921: Add borrowernumber and branchcode as html attributes To test: 1 - Apply patch 2 - Visit the opac 3 - Sign in 4 - You should not see any difference 5 - Inspect the element 'Welcome, {username}' 6 - Note the attributes for borrowernumber and branchcode 7 - Add to OPACUserJS: $(document).ready(function(){ console.log( $(".loggedinusername").attr('data-branchcode')); console.log( $(".loggedinusername").attr('data-borrowernumber')); }); 8 - Check the console and note you can see the info expected Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Created attachment 76413 [details] [review] Bug 20921: Add borrowernumber and branchcode as html attributes To test: 1 - Apply patch 2 - Visit the opac 3 - Sign in 4 - You should not see any difference 5 - Inspect the element 'Welcome, {username}' 6 - Note the attributes for borrowernumber and branchcode 7 - Add to OPACUserJS: $(document).ready(function(){ console.log( $(".loggedinusername").attr('data-branchcode')); console.log( $(".loggedinusername").attr('data-borrowernumber')); }); 8 - Check the console and note you can see the info expected Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 76414 [details] [review] Bug 20921: Fix opac_auth for selenium Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 76415 [details] [review] Bug 20921: Add selenium tests for .loggedinusername Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> I am not sure it makes sense to have borrowernumber and branchcode at loggedinusername level. Maybe we should rename it "loggedinuser"? Can be done later. Awesome work all! Pushed to master for 18.11 Enhancement, won't be backported to 18.05.x series. |