|
Description
Liz Rea
2014-01-08 01:59:27 UTC
Further review shows that the notes are stored in the database, but are not displayed properly. The interface seems to suggest that changing the dropdown would show the list for that particular issue (and it's notes, if it has some). That does not appear to be the case -- for notes, the actual membership of the list I have not checked on (yet). In fact, it looks like it shows the notes for the most recent received serial only, and switching the volume doesn't change the data displayed. Created attachment 82945 [details] [review] Bug 11492: Keep routing notes when receiving next serial This patch ensures the routing notes are carried over when generating the next serial. To test: 1) Create a routing list for a subscription 2) Add a borrower and a note to the routing list 3) Generate the next serial (serials-collection.pl) 4) Edit the routing list to see the notes 5) Note that the notes have disappeared 6) Apply patch 7) Edit the routing list, add a note 8) Generate the next serial 9) Edit the routing list and confirm the note is still there 10) Confirm you are still able to edit serials (serials-edit.pl) and routing notes stay Sponsored-by: Plant and Food Research Limited Created attachment 82946 [details] [review] Bug 11492: Keep routing notes when receiving next serial This patch ensures the routing notes are carried over when generating the next serial. To test: 1) Create a routing list for a subscription 2) Add a borrower and a note to the routing list 3) Generate the next serial (serials-collection.pl) 4) Edit the routing list to see the notes 5) Note that the notes have disappeared 6) Apply patch 7) Edit the routing list, add a note 8) Generate the next serial 9) Edit the routing list and confirm the note is still there 10) Confirm you are still able to edit serials (serials-edit.pl) and routing notes stay Sponsored-by: Plant and Food Research Limited Created attachment 83036 [details] [review] Bug 11492: Keep routing notes when receiving next serial This patch ensures the routing notes are carried over when generating the next serial. To test: 1) Create a routing list for a subscription 2) Add a borrower and a note to the routing list 3) Generate the next serial (serials-collection.pl) 4) Edit the routing list to see the notes 5) Note that the notes have disappeared 6) Apply patch 7) Edit the routing list, add a note 8) Generate the next serial 9) Edit the routing list and confirm the note is still there 10) Confirm you are still able to edit serials (serials-edit.pl) and routing notes stay Sponsored-by: Plant and Food Research Limited Signed-off-by: Devinim <kohadevinim@devinim.com.tr> Hi Aleisha, You are doing things that are not advertised in the commit message. For instance you are passing $publisheddatetext to NewIssue. I feel like this is wrong, $publisheddatetext should not be kept from one issue to another. There is 1160 my $nextpublisheddate = GetNextDate($subscription, $publisheddate, 1); 1161 my $nextpubdate = $nextpublisheddate; And $nextpubdate is passed as $planneddate and $publisheddate for the next one, but $publisheddatetext will be unchanged. (In reply to Jonathan Druart from comment #5) > Hi Aleisha, > > You are doing things that are not advertised in the commit message. For > instance you are passing $publisheddatetext to NewIssue. I feel like this is > wrong, $publisheddatetext should not be kept from one issue to another. > > There is > 1160 my $nextpublisheddate = GetNextDate($subscription, > $publisheddate, 1); > 1161 my $nextpubdate = $nextpublisheddate; > > And $nextpubdate is passed as $planneddate and $publisheddate for the next > one, but $publisheddatetext will be unchanged. Hi Jonathan, I only did this because the subroutine already took $publisheddatetext as a parameter, so thought it would be okay. I will resubmit my patch with an empty string in the place of $publisheddatetext, but otherwise this seems like a different problem. Created attachment 83192 [details] [review] Bug 11492: (follow-up) Don't pass publisheddatetext param to NewIssue I have removed publisheddatetext as a param from my patch, but only in the places where my patch added it. Any other instance already existed before my patch. Created attachment 89675 [details] [review] Bug 11492: Keep routing notes when receiving next serial This patch ensures the routing notes are carried over when generating the next serial. To test: 1) Create a routing list for a subscription 2) Add a borrower and a note to the routing list 3) Generate the next serial (serials-collection.pl) 4) Edit the routing list to see the notes 5) Note that the notes have disappeared 6) Apply patch 7) Edit the routing list, add a note 8) Generate the next serial 9) Edit the routing list and confirm the note is still there 10) Confirm you are still able to edit serials (serials-edit.pl) and routing notes stay Sponsored-by: Plant and Food Research Limited Signed-off-by: Nazlı Çetin <nazli@devinim.com.tr> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Tested on top of 22812 because docker runs in strict mode. Created attachment 89676 [details] [review] Bug 11492: (follow-up) Don't pass publisheddatetext param to NewIssue I have removed publisheddatetext as a param from my patch, but only in the places where my patch added it. Any other instance already existed before my patch. Signed-off-by: Liz Rea <wizzyrea@gmail.com> QA: Looking here now Hmm I'm having the impression that the whole issue field on the edit routing list form is kind of useless. The notes are written to all serial records, and the borrowers are kept on routing list level. Strange design ;) Created attachment 90214 [details] [review] Bug 11492: Keep routing notes when receiving next serial This patch ensures the routing notes are carried over when generating the next serial. To test: 1) Create a routing list for a subscription 2) Add a borrower and a note to the routing list 3) Generate the next serial (serials-collection.pl) 4) Edit the routing list to see the notes 5) Note that the notes have disappeared 6) Apply patch 7) Edit the routing list, add a note 8) Generate the next serial 9) Edit the routing list and confirm the note is still there 10) Confirm you are still able to edit serials (serials-edit.pl) and routing notes stay Sponsored-by: Plant and Food Research Limited Signed-off-by: Nazlı Çetin <nazli@devinim.com.tr> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 90215 [details] [review] Bug 11492: (follow-up) Don't pass publisheddatetext param to NewIssue I have removed publisheddatetext as a param from my patch, but only in the places where my patch added it. Any other instance already existed before my patch. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.01 backported to 18.11.x for 18.11.07 |