Bug 21772 - Add holds and account information tab to the SCO module
Summary: Add holds and account information tab to the SCO module
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Agustín Moyano
QA Contact: Josef Moravec
URL:
Keywords:
: 9047 (view as bug list)
Depends on:
Blocks: 12166 22538 22588
  Show dependency treegraph
 
Reported: 2018-11-06 15:07 UTC by Jesse Maseto
Modified: 2019-07-17 16:01 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21772: Move OPAC holds and account tables into .inc (48.93 KB, patch)
2019-03-07 23:08 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 21772: (follow-up) QA failed because old bootstrap button class (1.67 KB, patch)
2019-03-07 23:08 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 21772: Add holds and account information to SCO (19.21 KB, patch)
2019-03-07 23:08 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 21772: (follow-up) Remove redundant variable (803 bytes, patch)
2019-03-07 23:09 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 21772: (follow-up) Included more information on tab label for SCO (1.66 KB, patch)
2019-03-12 01:59 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 21772: Move OPAC holds and account tables into .inc (49.05 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: Add holds and account information to SCO (19.32 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (follow-up) Remove redundant variable (921 bytes, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (follow-up) Included more information on tab label for SCO (1.78 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (QA follow-up) Hide .tdlabel class in sco for consistancy with opac-user page. (1.05 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (QA follow-up) Fix button class on Cancel hold button (1.56 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (QA follow-up) Fix setting variables to PROCESS in holds (1.33 KB, patch)
2019-03-27 08:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21772: (follow-up) Minor markup correction and reindentation (24.27 KB, patch)
2019-03-27 12:36 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21772: (follow-up) Minor markup correction and reindentation (24.34 KB, patch)
2019-03-27 13:18 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Maseto 2018-11-06 15:07:56 UTC
Work to be:
1) Move opac-user.pl holds tab content into include file
2) Add holds to to sco, use new include file
3) Add patron fines info to sco.
4) Modify script to search for and pass in patron holds
Comment 1 Agustín Moyano 2019-03-07 23:08:50 UTC Comment hidden (obsolete)
Comment 2 Agustín Moyano 2019-03-07 23:08:53 UTC Comment hidden (obsolete)
Comment 3 Agustín Moyano 2019-03-07 23:08:56 UTC Comment hidden (obsolete)
Comment 4 Agustín Moyano 2019-03-07 23:09:00 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2019-03-08 09:54:36 UTC
*** Bug 9047 has been marked as a duplicate of this bug. ***
Comment 6 Sarah Cornell 2019-03-11 18:36:35 UTC
Agustín,

I tested the patch and the functionality works.  Is it possible to include more information in the tab labels in SCO, as it is displayed in opac-user.pl? 

Such as: 

Checkouts (0) Holds (1) Fines and charges (16.99) 


Thanks,

Sarah
Comment 7 Agustín Moyano 2019-03-12 01:59:00 UTC Comment hidden (obsolete)
Comment 8 Sarah Cornell 2019-03-12 21:28:08 UTC
The tabs look good.  I'm almost ready to sign off.  

At some point in my testing the SCO shut down.  I think it was because the SCO was trying to edit an item as part of the normal process.  I reproduced the shutdown when I tried to edit an item in the intranet.  This is one of the last lines of the plack log when it shuts down:

Use of uninitialized value $searchid in concatenation (.) or string at /kohadevbox/koha/cataloguing/addbiblio.pl line 853.
2019/03/12-19:48:38 Server closing!

I don't see why it would be related but I'm new to testing on the sandboxes so I have to ask!
Comment 9 Agustín Moyano 2019-03-14 10:44:10 UTC
Hello Sarah,

I believe that behaviour isn't related with this bug.

I think somehow SCO and intranet are overwritting the others browser cookies.

Please try opening each one in different browsers, or one in incognito mode.

Thanks
Comment 10 Sarah Cornell 2019-03-14 14:02:28 UTC
I got the same behavior (tab closes when invoking additem.pl) when testing another bug in the sandboxes, even after clearing cookies and only using the intranet.  So I guess it is unrelated to this bug.  

I'm signing off because your patches work. If this isn't the right thing to do, I'm sure someone will let me know!
Comment 11 Josef Moravec 2019-03-27 08:26:33 UTC
Comment on attachment 86332 [details] [review]
Bug 21772: (follow-up) QA failed because old bootstrap button class

This is false positive. In opac, we still use class btn-mini as Bootstrap in opac was not upgraded to version 3 and is still using version 2
Comment 12 Josef Moravec 2019-03-27 08:48:19 UTC
Created attachment 87042 [details] [review]
Bug 21772: Move OPAC holds and account tables into .inc

This patch moves the code to render the account and holds information
for the patron into include files. They are loaded in the opac-user.tt
and opac-account.tt templates respectively.

To test:
1) Have a patron with account lines and holds
2) Notice they are displayed in the patron's detail section in the OPAC
3) Apply this patch
4) Reload the patron's detail page
=> SUCESS: Everything looks the same, no behaviuor or layout change.
5) Sign off

Sponsored-by: City of Portsmouth Public Library

Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Josef Moravec 2019-03-27 08:48:23 UTC
Created attachment 87043 [details] [review]
Bug 21772: Add holds and account information to SCO

This patch introduces holds and account information to the SCO page. It
uses the introduced include files that are also used in OPAC
(opac-user.pl and opac-account.pl).

To test:
1) Have a patron that has holds and account lines
2) Enable the SCO module
3) Use the patron's cardnumber to get to its details
=> SUCCESS: New tabs are presented, and they display holds and account
information as expected
4) Sign off :-D

Sponsored-by: City of Portsmouth Public Library

Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 14 Josef Moravec 2019-03-27 08:48:26 UTC
Created attachment 87044 [details] [review]
Bug 21772: (follow-up) Remove redundant variable

Sponsored-by: City of Portsmouth Public Library

Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Josef Moravec 2019-03-27 08:48:30 UTC
Created attachment 87045 [details] [review]
Bug 21772: (follow-up) Included more information on tab label for SCO

Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2019-03-27 08:48:33 UTC
Created attachment 87046 [details] [review]
Bug 21772: (QA follow-up) Hide .tdlabel class in sco for consistancy with opac-user page.

Test plan:
1) Compare holds table on selfcheckout and opac-user page, you will see
the difference: In self checkout there are labels like Status, Pickup
location, and so on in every table cell.
2) Apply the patch, the label in selfcheckout dissappear

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Josef Moravec 2019-03-27 08:48:36 UTC
Created attachment 87047 [details] [review]
Bug 21772: (QA follow-up) Fix button class on Cancel hold button

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 18 Josef Moravec 2019-03-27 08:48:40 UTC
Created attachment 87048 [details] [review]
Bug 21772: (QA follow-up) Fix setting variables to PROCESS in holds

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Owen Leonard 2019-03-27 12:36:56 UTC
Created attachment 87064 [details] [review]
Bug 21772: (follow-up) Minor markup correction and reindentation

This patch makes one minor markup correction: In HTML5 <tfoot> must be
after <tbody>. Previously the opposite was true.

I've also done a little bit of opinionated reindentation. There should
be no changes visible to the user from this patch.
Comment 20 Owen Leonard 2019-03-27 12:37:32 UTC
I hope you don't mind the minor follow-up. I can submit on a separate bug if that's better.
Comment 21 Josef Moravec 2019-03-27 13:18:19 UTC
Created attachment 87067 [details] [review]
Bug 21772: (follow-up) Minor markup correction and reindentation

This patch makes one minor markup correction: In HTML5 <tfoot> must be
after <tbody>. Previously the opposite was true.

I've also done a little bit of opinionated reindentation. There should
be no changes visible to the user from this patch.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Josef Moravec 2019-03-27 13:18:51 UTC
(In reply to Owen Leonard from comment #20)
> I hope you don't mind the minor follow-up. I can submit on a separate bug if
> that's better.

That's OK.
Comment 23 Agustín Moyano 2019-03-27 17:38:41 UTC
(In reply to Owen Leonard from comment #20)
> I hope you don't mind the minor follow-up. I can submit on a separate bug if
> that's better.

Looks great, thanks :D
Comment 24 Nick Clemens 2019-03-28 18:19:20 UTC
Awesome work all!

Pushed to master for 19.05
Comment 25 Martin Renvoize 2019-04-01 14:00:17 UTC
Enhancement, will not be backported to 18.11.x series.