Created attachment 28754 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page.
I still have concerns about overloading the tables with information - see my comment on bug 11865. I think only very few libraries are using the granularity of permissions, owners and creators that acq offers right now.
That's why I moved it in a popup. It doesn't take extra space on screen. I don't see where is the problem now.
By the way, this has nothing to do with permissions of the acq module. Our customer only wants to see "who has created this order line", which is not possible actually in current master.
Hi Julian, I will take a look :)
(In reply to Katrin Fischer from comment #2) > I still have concerns about overloading the tables with information - see my > comment on bug 11865. I am not sure this is relevant. I submitted a patch for that (bug 10212). Everyone will be able to display, for each page, the columns they want - and this only with a small piece of code.
I've applied against master 3.17.00.007 At 6-th step: 6/ Your name should appear in front of "Created by" label, to the right of the page. an error occurs, because, on the "Accounting details" page on the right, under "Fund" label, in front of "Created by" label I've seen: Created by: No name The error appears when the pref "AcqCreateItem" is either "placing an order" or "receiving an order". So I pass the patch to "Failed QA" status. Note-1. Perhaps there is an error on master. On Comment 1 : >On acqui/orderreceive.pl it replace the name of basket's creator on master I don't see the name of basket's creator On acqui/orderreceive.pl. On the contrary I see: Created by: No name Note-2. On parcel.pl, The "View record" column is either in the "Pending orders" upper table, or in the "Already received" lower one.
Hi Paola, have you created the order line being logged in with a real user (not the mysql user) ? Does this user have a firstname and a surname ?
(In reply to Julian Maurice from comment #8) > Hi Paola, > > have you created the order line being logged in with a real user (not the > mysql user) ? Does this user have a firstname and a surname ? Thanks, Julian. Today I've applied against master 3.17.00.008. I've added surname, first name, aso to my test-user, the creator of the orders. Now everything's OK. So I pass the patch to "Signed Off" status.
Created attachment 28943 [details] [review] Bug 12395: Save order line's creator
Created attachment 30128 [details] [review] [PASSED QA] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 30129 [details] [review] Bug 12395 [QA Followup] * Rename createdby to created_by * Fix GPL versioning
I suspect a regression: For existing baskets, the created_by value is not set. Maybe should we add a sql query to set this field to the authorisedby value? Julian, could you confirm, or not?
Created attachment 31409 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 31410 [details] [review] Bug 12395 [QA Followup] * Rename createdby to created_by * Fix GPL versioning
Created attachment 31411 [details] [review] Bug 12395: Update aqorders.created_by with aqbasket.authorisedby Before this patch, the interface assumed the basket's creator was the "authorisedby" person. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
I was going to push this patch set to the MM acq branch. But since I added a follow-up, I prefer that RM push them.
The problem is the change to NewOder, that has since been rewriten and moved to Koha::Orders: + if (not defined $orderinfo->{createdby}) { + my $userenv = C4::Context->userenv; + if ($userenv) { + $orderinfo->{createdby} = $userenv->{number}; + } + } +
Created attachment 33837 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 33838 [details] [review] Bug 12395 [QA Followup] * Rename createdby to created_by * Fix GPL versioning
Created attachment 33839 [details] [review] Bug 12395: Update aqorders.created_by with aqbasket.authorisedby Before this patch, the interface assumed the basket's creator was the "authorisedby" person. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Julian, Could you please take a look: I tried to rebase it but it does not seem to work. It would be great to have a couple of tests in t/db_dependent/Acquisition/NewOrder.t
Created attachment 35769 [details] [review] Bug 12395: Save order line's creator Rebased on master
Created attachment 35770 [details] [review] Bug 12395 [QA Followup] Rebased on master
Created attachment 35771 [details] [review] Bug 12395: Update aqorders.created_by with aqbasket.authorisedby Rebased on master
Created attachment 35772 [details] [review] Bug 12395: Add unit test to check created_by is correctly set
Patches rebased on master + new unit test
I've tried to apply the patch against master 3.19.00.010 head 13675 Applying: Bug 12395: Save order line's creator Using index info to reconstruct a base tree... <stdin>:234: new blank line at EOF. + warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/kohastructure.sql Failed to merge in the changes. Patch failed at 0001 Bug 12395: Save order line's creator So I pass the patch to "Patch doesn't apply" status. NB. All the first 3 patches relate to updatedatabase.
Created attachment 35779 [details] [review] Bug 12395: Save order line's creator Rebased on master
Created attachment 35780 [details] [review] Bug 12395 [QA Followup] Rebased on master
Created attachment 35781 [details] [review] Bug 12395: Update aqorders.created_by with aqbasket.authorisedby Rebased on master
Created attachment 35782 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Rebased on master
Thanks, Julian. I've applied the patches against master 3.19.00.010 head 13240. After having run updatedatabase, the new "created_by" field of the already existing aqorders was set to the basket's creator code, as expected. On acqui/parcel.pl, in "More" column, for each already existing aqorder, on "Order" link this Creator's name appeared, as required. On acqui/orderreceive.pl, receiving an already existing order, I saw this Creator's name in front of "Created by:" label, as required. For new orders, in DB the "created_by" field of aqorder record was set to NULL (ERROR). On acqui/parcel.pl, in "More" column, on "Order" link I saw none "Creator" (ERROR). On acqui/orderreceive.pl, I saw "No name" in front of "Created by:" label, to the right of the page (ERROR). So I pass the patch to "Failed QA" status.
It works for me. Did you run misc/devel/update_dbix_class_files.pl ? Are you logged in with a real user (not the database user) ?
(In reply to Julian Maurice from comment #34) > Are you logged in with a real user (not the database user) ? The user was not a database user [it was the same user OK on already-existing-order]. > Did you run misc/devel/update_dbix_class_files.pl ? Thanks, Julian, I run the command but the problem was not solved. So I keep the status "Failed QA". [NB. If the above command be required, the test plan could be updated about it.] Now the already-existing-order had not the Creator's name on receiving (ERROR). [The already-existing-order' More was still OK.] See also: prove t/db_dependent/Acquisition/NewOrder.t t/db_dependent/Acquisition/NewOrder.t .. 1/8 # Failed test 'Koha::Acquisition::Order->insert set created_by to logged in user if not given' # at t/db_dependent/Acquisition/NewOrder.t line 91. # got: undef # expected: '42' # Looks like you failed 1 test of 8. t/db_dependent/Acquisition/NewOrder.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/8 subtests Test Summary Report ------------------- t/db_dependent/Acquisition/NewOrder.t (Wstat: 256 Tests: 8 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=8, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.83 cusr 0.08 csys = 0.92 CPU) Result: FAIL I hope this error can help.
Created attachment 35864 [details] [review] Bug 12395: Fix call to set_userenv in unit tests
This patch should fix the test. I don't understand why it doesn't work for you... :/
> Now the already-existing-order had not the Creator's name on receiving > (ERROR). [The already-existing-order' More was still OK.] And this is weird, because both use the same data...
(In reply to Julian Maurice from comment #36) > Created attachment 35864 [details] [review] [review] > Bug 12395: Fix call to set_userenv in unit tests Sorry, the 5-th patch doesn't fix: prove t/db_dependent/Acquisition/NewOrder.t t/db_dependent/Acquisition/NewOrder.t .. 1/8 # Failed test 'Koha::Acquisition::Order->insert set created_by to logged in user if not given' # at t/db_dependent/Acquisition/NewOrder.t line 91. # got: undef # expected: '42' # Looks like you failed 1 test of 8. t/db_dependent/Acquisition/NewOrder.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/8 subtests Test Summary Report ------------------- t/db_dependent/Acquisition/NewOrder.t (Wstat: 256 Tests: 8 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=8, 1 wallclock secs ( 0.00 usr 0.01 sys + 0.83 cusr 0.07 csys = 0.91 CPU) Result: FAIL
Hmm... could you check that you have the aqorders.created_by field in your database and that Koha/Schema/Result/Aqorder.pm contains an entry about created_by ?
Thank you very much, Julian! [in the meanwhile I've installed some not-required perl modules.] (In reply to Julian Maurice from comment #40): >could you check that Koha/Schema/Result/Aqorder.pm contains an entry about > created_by ? yes, it does! The file Koha/Schema/Result/Aqorder.pm in my branch has been updated: cat -n Koha/Schema/Result/Aqorder.pm | grep 'created_by' 97 =head2 created_by 273 "created_by", >could you check that you have the aqorders.created_by field in your > database ? yes, I have; and now the "created_by" field is OK, either for existing orders or for new orders! Now the "More" option is OK either for existing orders or for new orders! Whilst the orderreceive.pl still shows "Created by: No name" in both the cases of orders (ERROR).
So you have the correct borrowernumber in aqorders.created_by, but "No name" is displayed on orderreceive.pl ?
I've applied the patches against master 3.19.00.010 head 13189. I've upgraded the DB. I've run misc/devel/update_dbix_class_files.pl : it was OK. The DB was OK for existing and new orders. On parcel.pl the "More" Order was OK for existing and new orders. (In reply to Julian Maurice from comment #42) > So you have the correct borrowernumber in aqorders.created_by, yes, >but "No name" is displayed on orderreceive.pl ? yes.
Created attachment 36063 [details] [review] Bug 12395: Display created_by even if surname is missing on receiving On receiving orders, "no name" was displayed if the surname of creator was not filled. The firstname should be displayed.
Paola, I think I have found your problem. If the firstname (or surname) was not filled, "no name" was display on this screen. The last patch should fix that.
(In reply to Jonathan Druart from comment #45) > Paola, I think I have found your problem. > If the firstname (or surname) was not filled, "no name" was display on this > screen. > The last patch should fix that. Thanks, Jonathan. It fixes!
(In reply to Jonathan Druart from comment #44) > Created attachment 36063 [details] [review] [review] > Bug 12395: Display created_by even if surname is missing on receiving > > On receiving orders, "no name" was displayed if the surname of creator > was not filled. > The firstname should be displayed. The surname is mandatory ("Required") for a patron/borrower/creator. So I've changed the patch's subject to: Bug 12395: Display created_by even if firstname is missing on receiving > > On receiving orders, "no name" was displayed if the firstname of creator > was not filled. > The surname should be displayed.
Created attachment 36075 [details] [review] Bug 12395: Save order line's creator Thanks, Julian and Jonathan. I've applied the patches against master 3.19.00.011 head 13728. I've upgraded the DB. Then I've run misc/devel/update_dbix_class_files.pl on my branch. Everything was OK as required by the test plan (see comment #1). So I pass the patch to "Signed Off" status.
Created attachment 36076 [details] [review] Bug 12395 [QA Followup]
Created attachment 36077 [details] [review] Bug 12395: Update aqorders.created_by with aqbasket.authorisedby
Created attachment 36078 [details] [review] Bug 12395: Add unit test to check created_by is correctly set
Created attachment 36079 [details] [review] Bug 12395: Fix call to set_userenv in unit tests
Created attachment 36080 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving
Starting on this... - QA script OK - Code review OK ------------------- 1) New test is failing for me: not ok 8 - Koha::Acquisition::Order->insert set created_by to logged in user if not given # Failed test 'Koha::Acquisition::Order->insert set created_by to logged in user if not given' # at t/db_dependent/Acquisition/NewOrder.t line 91. # got: undef # expected: '42' # Looks like you failed 1 test of 8. 2) Why only display the order line creator during the receive process, but not on the order pages? We can only see the basket creator there. 3) On the order receive page: Maybe we should write "Ordered by" instead of "Created by"? 4) On the parcel page: You change the display for the unreceived order lines table, but the received orders miss the link to the order. I think it would be better to be consistent and also to keep the | for separating the links from each other. 5) The popup window appers 'unstyled' - for example it appears in a much bigger than the rest serif font for me. Maybe something for a follow up? Could you please take a look? Especially at the failing test and 4)
Created attachment 40132 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update.
Created attachment 40133 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 40134 [details] [review] Bug 12395: Fix call to set_userenv in unit tests Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 40135 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 40136 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update.
Created attachment 40137 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 40138 [details] [review] Bug 12395: Fix call to set_userenv in unit tests Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 40139 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
(In reply to Katrin Fischer from comment #54) > Starting on this... > > - QA script OK > - Code review OK > > ------------------- > > 1) New test is failing for me: > not ok 8 - Koha::Acquisition::Order->insert set created_by to logged in user > if not given > # Failed test 'Koha::Acquisition::Order->insert set created_by to logged > in user if not given' > # at t/db_dependent/Acquisition/NewOrder.t line 91. > # got: undef > # expected: '42' > # Looks like you failed 1 test of 8. Katrin, this typically occurs when "created_by" key is missing in Koha/Schema/Result/Aqorder.pm. Did you run misc/devel/update_dbix_class_files.pl ? > 2) Why only display the order line creator during the receive process, but > not on the order pages? We can only see the basket creator there. I'm not sur to understand where you want to add the creator again. Is it in basket.pl ?
Hm, I think it would have to be basket.pl currently - but we already got a lot of information there. I think it's not a blocker. What I could think of as enh: - create a "view" page for an order line that is still accessible after the basket has been closed (similar to suggestions) and show it there. - add colvis to the basket page, so the library can decide if it wants to display it or not.
> What I could think of as enh: > - create a "view" page for an order line that is still accessible after the > basket has been closed (similar to suggestions) and show it there. Could ascqui/showorder.pl be this view page ?
Created attachment 40161 [details] [review] [Follow-up] Bug 12395 - Make received orders table consistent with pending orders table (links of More column), - Change "Created by" with "Ordered by" in receive page.
I think the last new patch is ok as a QA follow-up - shifting back to signed off.
Created attachment 43951 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag and fix title description - Fixes title attribute for some of the new links - Adds a unique id and a class to the new modal (xt/tt_valid.t)
1) xt/tt_valid.t was failing, I fixed it in my follo-up, please check. 1..3 ok 1 - TT syntax: not using TT directive within HTML tag not ok 2 - <body> tag with id and class attributes # Failed test '<body> tag with id and class attributes' # at xt/tt_valid.t line 91. # Files list: # intranet-tmpl/prog/en/modules/acqui/showorder.tt: 12 ok 3 - Token received same as original put on stack # Looks like you failed 1 test of 3. 2) Some of the title="MARC" should be title="Card" - fixed in my follow-up, please check. 3) t/db_dependent/Acquisition/NewOrder.t fails, please fix (blocker) :( I think it's not only the test - in my testing the creator is not saved for newly created orders. Only the 'migrated' orders have created_by set in the database. 1..8 ok 1 - Inserting an order with no params returns undef and croaks ok 2 - Inserting an order with no budget_id returns undef and croaks with expected error message ok 3 - Inserting an order with no biblionumber returns undef and croaks with expected error message ok 4 - Inserting an order with no basketno returns undef and croaks with expected error message ok 5 - Inserting an order with no quantity returns undef and croaks with expected error message ok 6 - Koha::Acquisition::Order->insert set quantityreceivedto 0 if undef is given ok 7 - Koha::Acquisition::Order->insert set entrydate to today not ok 8 - Koha::Acquisition::Order->insert set created_by to logged in user if not given # Failed test 'Koha::Acquisition::Order->insert set created_by to logged in user if not given' # at t/db_dependent/Acquisition/NewOrder.t line 93. # got: undef # expected: '42' # Looks like you failed 1 test of 8. 4) The 'look' of the new modal for order information is slightly off, especially font and font-size are unusual. Would be nice to be more consistent with Koha's look and feel here.
3) For the tests to pass you need to run misc/devel/update_dbix_class_files.pl after updatedatabase.pl 4) I don't really know what to do here... Is there another modal window that we can take as an example ?
4) I think using the standard font and font-size would already help - it looked very different than the page in the background to me.
Created attachment 45387 [details] [review] Bug 12395: QA follow-up - Use Bootstrap modal instead of Greybox - Using Bootstrap modals reduces unnecessary dependencies and allows for more consistent styling (contents of the modal are styled by the current page's CSS). - Using standard <fieldset> + <ol> markup for showorder.tt.
on the order receive page (http://pro.test2.biblibre.com/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=7&invoiceid=3) there's Ordered by: No name ...
Created attachment 58745 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update.
Created attachment 58746 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 58747 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 58748 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page.
Created attachment 58749 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag
Created attachment 58750 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt
Rebased on master. Bug 16592 already added the bootstrap modal to the parcel.pl page so I removed the related code from this bug's patches.
Created attachment 61383 [details] column more > order
Created attachment 61384 [details] no name when receving
Tested on sandbox biblibre 5 : The name is never printed at : http://pro.user05-koha.sandbox.biblibre.eu/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=8&invoiceid=2 (receiving order) nor at : http://pro.user05-koha.sandbox.biblibre.eu/cgi-bin/koha/acqui/parcel.pl?invoiceid=3 (pop up when click on more > order) Please, see screenshots attached.
Created attachment 71344 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update.
Created attachment 71345 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 71346 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Created attachment 71347 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page.
Created attachment 71348 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag
Created attachment 71349 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt
Patches rebased on master
Created attachment 72720 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72721 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72722 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72723 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72724 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 72725 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Patch doesn't apply, please rebase and check for changes needed.
Created attachment 74207 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74208 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74209 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74210 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74211 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74212 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
1) Should showorder.pl, I think it should be: use CGI qw ( -utf8 ); 2) I was wondering why the order lines creator didn't show on /cgi-bin/koha/acqui/basket.pl?basketno=1. Neither does it when editing an order line. Maybe a follow-up (separate bug?) 3) Make both 'Order' information and orderreceive.pl use 'Ordered by'. Nothing else. I'll fix 1) and 3) in a follow-up. Please check!
Created attachment 74276 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74277 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74278 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74279 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74280 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74281 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74282 [details] [review] Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI;
Bug 12395: Save order line's creator reverts bug 20303
2. Why not a foreign key on aqorders.created_by (with on delete set null)? 3. orderreceive.tt and showorder.tt must use patron-title.inc
Created attachment 75900 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75901 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75902 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75903 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75904 [details] [review] Bug 12395: QA follow-up - Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75905 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75906 [details] [review] Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 75907 [details] [review] Bug 12395: Add FK constraint and use patron-title.inc Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
(In reply to Jonathan Druart from comment #114) > 2. Why not a foreign key on aqorders.created_by (with on delete set null)? > > 3. orderreceive.tt and showorder.tt must use patron-title.inc Fixed in the last patch
(In reply to Jonathan Druart from comment #113) > Bug 12395: Save order line's creator > reverts bug 20303 This one is still missing I think.
(In reply to Jonathan Druart from comment #124) > (In reply to Jonathan Druart from comment #113) > > Bug 12395: Save order line's creator > > reverts bug 20303 > > This one is still missing I think. These changes are intended. orderreceive.pl doesn't use aqbaskets.authorisedby anymore (with those patches applied)
Created attachment 76074 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76075 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76076 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76077 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76078 [details] [review] Bug 12395: (QA follow-up) Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76079 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76080 [details] [review] Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 76081 [details] [review] Bug 12395: Add FK constraint and use patron-title.inc Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
(In reply to Julian Maurice from comment #125) > (In reply to Jonathan Druart from comment #124) > > (In reply to Jonathan Druart from comment #113) > > > Bug 12395: Save order line's creator > > > reverts bug 20303 > > > > This one is still missing I think. > > These changes are intended. orderreceive.pl doesn't use > aqbaskets.authorisedby anymore (with those patches applied) Indeed! I have tested to delete the account of the creator and receive an order and it seems ok. The screen displays "Ordered by: " and nothing in front. Maybe we could add something to explain why it's empty.
Forgot the signed-off-by line, reattaching...
Created attachment 76082 [details] [review] Bug 12395: Save order line's creator New MySQL column: aqorders.createdby Creator's name is displayed on order's receive pages (acqui/orderreceive.pl and acqui/parcel.pl) On acqui/orderreceive.pl it replace the name of basket's creator On acqui/parcel.pl, to avoid adding more data in the table of pending orders, it is shown in a popup like MARC and Card views Test plan: 1/ Run updatedatabase.pl 2/ Create a new order and go to the receipt page (acqui/parcel.pl) 3/ Click on "Order" link in column "More" (previously "View record") 4/ A javascript popup should appear with your name in it. Close the popup. 5/ Click on "Receive" link 6/ Your name should appear in front of "Created by" label, to the right of the page. Patch updated with use of atomic update. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76083 [details] [review] Bug 12395: Add unit test to check created_by is correctly set Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76084 [details] [review] Bug 12395: Display created_by even if firstname is missing on receiving On receiving orders, "no name" was displayed if the firstname of creator was not filled. The surname should be displayed. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76085 [details] [review] Bug 12395: Make received orders table consistent with pending orders table (links of More column) Also change "Created by" with "Ordered by" in receive page. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76086 [details] [review] Bug 12395: (QA follow-up) Add class/id to body tag Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76087 [details] [review] Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76088 [details] [review] Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76089 [details] [review] Bug 12395: Add FK constraint and use patron-title.inc Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Awesome work all! Pushed to master for 18.11
Nice enhancement, not backporting to 18.05.x series.