|
Description
Katrin Fischer
2020-08-02 11:39:42 UTC
Created attachment 107664 [details] [review] Bug 26123: Add notice about new messages to the patron dashboard This patch adds a notice to the patron OPAC when a logged on user of the OPAC has 1 or more OPAC messages. Test plan: 1. Apply patch, restart_all. 2. Add some OPAC messages for a patron and login as that patron. 3. On the OPAC main page you should see a notice about your OPAC messages, incdicating how many you have. 4. Add multiple OPAC messages and make sure it all continues to work. 5. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Created attachment 107685 [details] [review] Bug 26123: Add notice about new messages to the patron dashboard This patch adds a notice to the patron OPAC when a logged on user of the OPAC has 1 or more OPAC messages. Test plan: 1. Apply patch, restart_all. 2. Add some OPAC messages for a patron and login as that patron. 3. On the OPAC main page you should see a notice about your OPAC messages, incdicating how many you have. 4. Add multiple OPAC messages and make sure it all continues to work. 5. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> This works well - is it possible to change the bug title? It seems to refer to the OPAC Note section (found when editing the patron), but this patch uses the Messages feature instead. Hi Lucas, could you please fix the missing filter and include the OPAC note as well? Also thanks for taking this on! Created attachment 108427 [details] [review] Bug 26123: Add notice about new messages to the patron dashboard This patch adds a notice to the patron OPAC when a logged on user of the OPAC has 1 or more OPAC messages, this includes both patron messages and the OPAC note. For privacy this doesn't show any of the messages/note on opac-main.tt. Instead it adds up the total number of patron messages + OPAC notes and displays that number in the dashboard Test plan: 1. Apply patch, restart_all. 2. Add some OPAC messages for a patron and login as that patron. 3. On the OPAC main page you should see a notice about your OPAC messages, incdicating how many you have. 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note, this should add 1 to the total number of messages on the dashboard. 6. Try an OPAC without any messages. 7. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. After applying the patch. If I am hitting http://localhost OPAC page without login. I am getting error. Software error: Can't call method "unblessed" on an undefined value at /home/amit/kohaclonemaster/opac/opac-main.pl line 79. Created attachment 109122 [details]
Error
QA comments
I think need to correction for ex:
-my $borr = $patron->unblessed;
- my $patron_note = $borr->{opacnote};
+ my $patron_note = $patron->opacnote;
Created attachment 109134 [details] [review] Bug 26123: Add OPAC note/patron message notice to dashboard I seperated OPAC note and patron message into two distinct categories with seperate notices for each. Slightly altered test plan Test plan: 1. Apply patch, restartgit 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refrsh the OPAC and you should see a notice like '1 note'. There is never more than a single OPAC note. 6. Try an OPAC without any messages. 7. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Amit, Thank you! I believe you were correct when suggesting: my $patron_note = $patron->opacnote; Would it be possible to combine the count of notes and messages? The distinction from the patron's point of view is meaningless. Notes and messages both displayed together in the same area on the user's page with no indication that they're different kinds of messages, so two different counts don't make sense. (In reply to Owen Leonard from comment #11) > Would it be possible to combine the count of notes and messages? The > distinction from the patron's point of view is meaningless. > > Notes and messages both displayed together in the same area on the user's > page with no indication that they're different kinds of messages, so two > different counts don't make sense. Owen, I was wondering about that too after I submitted the patch. I will combine them into one. Created attachment 111336 [details] [review] Bug 26123: Add message notification to patron dashboard Test plan: 1. Apply patch, restart_all 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refresh the OPAC and you should see a 1 message added to the total 6. Try an OPAC without any messages. 7. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Nice job.
Just my opinion :
Would be great to send Koha::Patron object to template.
So call in template [% patron.opacnote %].
Also add in Koha::Patron a method messages() to get : Koha::Patron::Messages->search({borrowernumber => $borrowernumber})
Like it exists for checkouts, holds ...
So in template you just add [% patron.messages.count %]
Hi Lucas, I think you need to limit the messages to the OPAC ones (they can be either for OPAC or staff only. Created attachment 112139 [details] [review] Bug 26123: Add message notification to patron dashboard Test plan: 1. Apply patch, restart_all 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refresh the OPAC and you should see a 1 message added to the total 6. Try an OPAC without any messages. 7. Add a 'staff - internal' note. It should not count towards your total messages in the dashboard. 8. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Created attachment 112227 [details] [review] Bug 26123: Add message notification to patron dashboard Test plan: 1. Apply patch, restart_all 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refresh the OPAC and you should see a 1 message added to the total 6. Try an OPAC without any messages. 7. Add a 'staff - internal' note. It should not count towards your total messages in the dashboard. 8. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Hi Lucas, I believe there is something wrong about this line here, as the empty box shows when no notes, checkouts, holds, etc are on the account now, when it should be hidden:
+ if ( $checkouts > 0 || $overdues_count > 0 || $holds_pending > 0 || $holds_waiting > 0 || $total > 0 || $patron_messages > 0 || $patron_note > 0 ) {
I suspect the $patron_messages > 0 might not be working. What do you think?
(In reply to Katrin Fischer from comment #18) > Hi Lucas, I believe there is something wrong about this line here, as the > empty box shows when no notes, checkouts, holds, etc are on the account now, > when it should be hidden: > > + if ( $checkouts > 0 || $overdues_count > 0 || $holds_pending > 0 || > $holds_waiting > 0 || $total > 0 || $patron_messages > 0 || $patron_note > 0 > ) { > > I suspect the $patron_messages > 0 might not be working. What do you think? Cait, you're right. I think 'patron_messages > 0' needs to be $patron_messages->count Also $patron_note, having only one possible note, can be $patron_note instead of $patron_note > 0. Created attachment 112519 [details] [review] Bug 26123: Add message notification to patron dashboard Test plan: 1. Apply patch, restart_all 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refresh the OPAC and you should see a 1 message added to the total 6. Try an OPAC without any messages. If there are no messages or any other kind of notification the dashboard should NOT appear at all. 7. Add a 'staff - internal' note. It should not count towards your total messages in the dashboard. 8. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> IIUC the dashboard will mention the messages until the opac note is cleared and/or the old messages are purged by bug 24541 for example. That's ok right? Even if it looks like a "new messages" feature, it's consistent with the patron page that will display them all, old or new. So it won't add much confusion. Other opinions on that? I've filed bug 27325 for a possible follow-up. No blocker here for me, as this patch follows the existing pattern and I think we should fix them all together which is out of scope here. Created attachment 114769 [details] [review] Bug 26123: Add message notification to patron dashboard Test plan: 1. Apply patch, restart_all 2. Add some OPAC messages by clicking 'Add a message' on Home › Circulation › Checkouts › patron 3. Log into the OPAC as that patron and you should see a notice about how many messages you have with a link to opac-user.pl 4. Add multiple OPAC messages and make sure it all continues to work. 5. Also add an OPAC note from memberentry.pl. Refresh the OPAC and you should see a 1 message added to the total 6. Try an OPAC without any messages. If there are no messages or any other kind of notification the dashboard should NOT appear at all. 7. Add a 'staff - internal' note. It should not count towards your total messages in the dashboard. 8. Test it with some other notices that would appear on the dashboard, checkouts, holds, overdues. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Is it ok to consider the "OPAC note" a message? (In reply to Jonathan Druart from comment #24) > Is it ok to consider the "OPAC note" a message? Yes, it prominently shows in the same spot as the "younger" messages feature. The OPAC note can be easily bulk added, while you can't bulk add the messages. Same for setting the note on patron import. So both options are used for messages to patrons still. Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x |