Bug 12395 - Save order line's creator
Summary: Save order line's creator
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Whiteboard:
Keywords: release-notes-needed
Depends on:
Blocks: 21129
  Show dependency treegraph
 
Reported: 2014-06-10 06:27 UTC by Julian Maurice
Modified: 2018-11-12 08:36 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Allow finer grained auditing of acquisition orders.


Attachments
Bug 12395: Save order line's creator (10.53 KB, patch)
2014-06-10 06:37 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.85 KB, patch)
2014-06-18 14:53 UTC, Paola Rossi
Details | Diff | Splinter Review
[PASSED QA] Bug 12395: Save order line's creator (10.65 KB, patch)
2014-07-25 15:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.11 KB, patch)
2014-07-25 15:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.61 KB, patch)
2014-09-05 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.09 KB, patch)
2014-09-05 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Update aqorders.created_by with aqbasket.authorisedby (1.15 KB, patch)
2014-09-05 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.41 KB, patch)
2014-11-24 11:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.05 KB, patch)
2014-11-24 11:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Update aqorders.created_by with aqbasket.authorisedby (1.15 KB, patch)
2014-11-24 11:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.43 KB, patch)
2015-02-10 09:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.05 KB, patch)
2015-02-10 09:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Update aqorders.created_by with aqbasket.authorisedby (1.15 KB, patch)
2015-02-10 09:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.55 KB, patch)
2015-02-10 09:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.43 KB, patch)
2015-02-10 13:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.05 KB, patch)
2015-02-10 13:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Update aqorders.created_by with aqbasket.authorisedby (1.15 KB, patch)
2015-02-10 13:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.55 KB, patch)
2015-02-10 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Fix call to set_userenv in unit tests (899 bytes, patch)
2015-02-12 11:50 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Display created_by even if surname is missing on receiving (1.24 KB, patch)
2015-02-20 10:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.71 KB, patch)
2015-02-20 12:57 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395 [QA Followup] (4.23 KB, patch)
2015-02-20 12:57 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395: Update aqorders.created_by with aqbasket.authorisedby (1.25 KB, patch)
2015-02-20 12:57 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.66 KB, patch)
2015-02-20 12:58 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395: Fix call to set_userenv in unit tests (990 bytes, patch)
2015-02-20 12:58 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.34 KB, patch)
2015-02-20 12:59 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.36 KB, patch)
2015-06-15 10:06 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.61 KB, patch)
2015-06-15 10:06 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Fix call to set_userenv in unit tests (956 bytes, patch)
2015-06-15 10:06 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.30 KB, patch)
2015-06-15 10:06 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.36 KB, patch)
2015-06-15 12:16 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.71 KB, patch)
2015-06-15 12:16 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Fix call to set_userenv in unit tests (960 bytes, patch)
2015-06-15 12:16 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.30 KB, patch)
2015-06-15 12:17 UTC, Alex Arnaud
Details | Diff | Splinter Review
[Follow-up] Bug 12395 (2.93 KB, patch)
2015-06-16 08:16 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag and fix title description (2.83 KB, patch)
2015-10-25 11:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Use Bootstrap modal instead of Greybox (7.45 KB, patch)
2015-12-03 19:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.22 KB, patch)
2017-01-10 13:19 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.64 KB, patch)
2017-01-10 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.30 KB, patch)
2017-01-10 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (2.79 KB, patch)
2017-01-10 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (780 bytes, patch)
2017-01-10 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.49 KB, patch)
2017-01-10 13:20 UTC, Julian Maurice
Details | Diff | Splinter Review
column more > order (65.43 KB, image/png)
2017-03-21 13:27 UTC, jmbroust
Details
no name when receving (60.48 KB, image/png)
2017-03-21 13:28 UTC, jmbroust
Details
Bug 12395: Save order line's creator (10.34 KB, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.62 KB, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.30 KB, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (2.80 KB, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (787 bytes, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.49 KB, patch)
2018-02-08 14:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.40 KB, patch)
2018-03-12 16:16 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.76 KB, patch)
2018-03-12 16:16 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.44 KB, patch)
2018-03-12 16:16 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (2.94 KB, patch)
2018-03-12 16:16 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (929 bytes, patch)
2018-03-12 16:17 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.63 KB, patch)
2018-03-12 16:17 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.59 KB, patch)
2018-04-16 08:22 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.77 KB, patch)
2018-04-16 08:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.45 KB, patch)
2018-04-16 08:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (2.98 KB, patch)
2018-04-16 08:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (936 bytes, patch)
2018-04-16 08:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.64 KB, patch)
2018-04-16 08:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.60 KB, patch)
2018-04-16 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.82 KB, patch)
2018-04-16 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.50 KB, patch)
2018-04-16 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (3.02 KB, patch)
2018-04-16 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (987 bytes, patch)
2018-04-16 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.69 KB, patch)
2018-04-16 21:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; (1.38 KB, patch)
2018-04-16 21:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.65 KB, patch)
2018-06-08 08:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.83 KB, patch)
2018-06-08 08:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.51 KB, patch)
2018-06-08 08:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (3.04 KB, patch)
2018-06-08 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: QA follow-up - Add class/id to body tag (994 bytes, patch)
2018-06-08 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.69 KB, patch)
2018-06-08 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; (1.45 KB, patch)
2018-06-08 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Add FK constraint and use patron-title.inc (3.82 KB, patch)
2018-06-08 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.65 KB, patch)
2018-06-14 21:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.83 KB, patch)
2018-06-14 21:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.51 KB, patch)
2018-06-14 21:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (3.04 KB, patch)
2018-06-14 21:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Add class/id to body tag (994 bytes, patch)
2018-06-14 21:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.69 KB, patch)
2018-06-14 21:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; (1.45 KB, patch)
2018-06-14 21:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Add FK constraint and use patron-title.inc (3.82 KB, patch)
2018-06-14 21:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Save order line's creator (10.72 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Add unit test to check created_by is correctly set (1.90 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Display created_by even if firstname is missing on receiving (1.58 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Make received orders table consistent with pending orders table (3.11 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Add class/id to body tag (1.04 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt (2.77 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI; (1.52 KB, patch)
2018-06-14 21:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12395: Add FK constraint and use patron-title.inc (3.89 KB, patch)
2018-06-14 21:12 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2014-06-10 06:27:08 UTC

    
Comment 1 Julian Maurice 2014-06-10 06:37:16 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2014-06-10 06:41:50 UTC
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.
Comment 3 Julian Maurice 2014-06-10 06:55:17 UTC
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.
Comment 4 Julian Maurice 2014-06-10 07:01:42 UTC
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.
Comment 5 Katrin Fischer 2014-06-10 09:33:19 UTC
Hi Julian, I will take a look :)
Comment 6 Jonathan Druart 2014-06-10 10:24:56 UTC
(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.
Comment 7 Paola Rossi 2014-06-13 12:37:48 UTC
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.
Comment 8 Julian Maurice 2014-06-16 07:47:08 UTC
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 ?
Comment 9 Paola Rossi 2014-06-18 14:52:50 UTC
(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.
Comment 10 Paola Rossi 2014-06-18 14:53:58 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2014-07-25 15:09:42 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2014-07-25 15:10:12 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-09-05 14:54:04 UTC
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?
Comment 14 Jonathan Druart 2014-09-05 15:24:09 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-09-05 15:24:29 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-09-05 15:24:34 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2014-09-05 15:25:32 UTC
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.
Comment 18 Katrin Fischer 2014-11-21 18:49:50 UTC
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};
+        }
+    }
+
Comment 19 Jonathan Druart 2014-11-24 11:33:39 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2014-11-24 11:33:44 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2014-11-24 11:33:49 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-11-24 11:35:05 UTC
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
Comment 23 Julian Maurice 2015-02-10 09:26:19 UTC Comment hidden (obsolete)
Comment 24 Julian Maurice 2015-02-10 09:26:29 UTC Comment hidden (obsolete)
Comment 25 Julian Maurice 2015-02-10 09:26:40 UTC Comment hidden (obsolete)
Comment 26 Julian Maurice 2015-02-10 09:26:47 UTC Comment hidden (obsolete)
Comment 27 Julian Maurice 2015-02-10 09:30:47 UTC
Patches rebased on master + new unit test
Comment 28 Paola Rossi 2015-02-10 13:14:06 UTC
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.
Comment 29 Julian Maurice 2015-02-10 13:31:33 UTC Comment hidden (obsolete)
Comment 30 Julian Maurice 2015-02-10 13:31:47 UTC Comment hidden (obsolete)
Comment 31 Julian Maurice 2015-02-10 13:31:59 UTC Comment hidden (obsolete)
Comment 32 Julian Maurice 2015-02-10 13:32:12 UTC Comment hidden (obsolete)
Comment 33 Paola Rossi 2015-02-11 16:01:13 UTC
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.
Comment 34 Julian Maurice 2015-02-12 08:53:12 UTC
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) ?
Comment 35 Paola Rossi 2015-02-12 11:23:44 UTC
(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.
Comment 36 Julian Maurice 2015-02-12 11:50:26 UTC Comment hidden (obsolete)
Comment 37 Julian Maurice 2015-02-12 11:56:55 UTC
This patch should fix the test.

I don't understand why it doesn't work for you... :/
Comment 38 Julian Maurice 2015-02-12 11:58:58 UTC
> 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...
Comment 39 Paola Rossi 2015-02-12 12:54:42 UTC
(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
Comment 40 Julian Maurice 2015-02-12 13:13:34 UTC
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 ?
Comment 41 Paola Rossi 2015-02-12 16:51:20 UTC
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).
Comment 42 Julian Maurice 2015-02-13 13:01:50 UTC
So you have the correct borrowernumber in aqorders.created_by, but "No name" is displayed on orderreceive.pl ?
Comment 43 Paola Rossi 2015-02-13 17:22:31 UTC
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.
Comment 44 Jonathan Druart 2015-02-20 10:33:56 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2015-02-20 10:35:01 UTC
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.
Comment 46 Paola Rossi 2015-02-20 12:49:19 UTC
(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!
Comment 47 Paola Rossi 2015-02-20 12:53:59 UTC
(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.
Comment 48 Paola Rossi 2015-02-20 12:57:02 UTC Comment hidden (obsolete)
Comment 49 Paola Rossi 2015-02-20 12:57:27 UTC Comment hidden (obsolete)
Comment 50 Paola Rossi 2015-02-20 12:57:52 UTC Comment hidden (obsolete)
Comment 51 Paola Rossi 2015-02-20 12:58:21 UTC Comment hidden (obsolete)
Comment 52 Paola Rossi 2015-02-20 12:58:45 UTC Comment hidden (obsolete)
Comment 53 Paola Rossi 2015-02-20 12:59:35 UTC Comment hidden (obsolete)
Comment 54 Katrin Fischer 2015-04-06 10:33:42 UTC
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)
Comment 55 Alex Arnaud 2015-06-15 10:06:26 UTC Comment hidden (obsolete)
Comment 56 Alex Arnaud 2015-06-15 10:06:34 UTC Comment hidden (obsolete)
Comment 57 Alex Arnaud 2015-06-15 10:06:42 UTC Comment hidden (obsolete)
Comment 58 Alex Arnaud 2015-06-15 10:06:48 UTC Comment hidden (obsolete)
Comment 59 Alex Arnaud 2015-06-15 12:16:47 UTC
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.
Comment 60 Alex Arnaud 2015-06-15 12:16:51 UTC
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>
Comment 61 Alex Arnaud 2015-06-15 12:16:55 UTC
Created attachment 40138 [details] [review]
Bug 12395: Fix call to set_userenv in unit tests

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Comment 62 Alex Arnaud 2015-06-15 12:17:01 UTC
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>
Comment 63 Alex Arnaud 2015-06-15 14:41:26 UTC
(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 ?
Comment 64 Katrin Fischer 2015-06-15 14:45:57 UTC
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.
Comment 65 Alex Arnaud 2015-06-15 15:14:57 UTC
> 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 ?
Comment 66 Alex Arnaud 2015-06-16 08:16:07 UTC
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.
Comment 67 Katrin Fischer 2015-07-09 12:11:31 UTC
I think the last new patch is ok as a QA follow-up - shifting back to signed off.
Comment 68 Katrin Fischer 2015-10-25 11:37:03 UTC
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)
Comment 69 Katrin Fischer 2015-10-25 11:37:41 UTC
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.
Comment 70 Julian Maurice 2015-11-09 15:49:17 UTC
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 ?
Comment 71 Katrin Fischer 2015-11-09 16:00:46 UTC
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.
Comment 72 Owen Leonard 2015-12-03 19:44:44 UTC
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.
Comment 73 Koha Team Lyon 3 2016-06-04 11:55:50 UTC
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 ...
Comment 74 Julian Maurice 2017-01-10 13:19:59 UTC
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.
Comment 75 Julian Maurice 2017-01-10 13:20:07 UTC
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>
Comment 76 Julian Maurice 2017-01-10 13:20:23 UTC
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>
Comment 77 Julian Maurice 2017-01-10 13:20:32 UTC
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.
Comment 78 Julian Maurice 2017-01-10 13:20:41 UTC
Created attachment 58749 [details] [review]
Bug 12395: QA follow-up - Add class/id to body tag
Comment 79 Julian Maurice 2017-01-10 13:20:49 UTC
Created attachment 58750 [details] [review]
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt
Comment 80 Julian Maurice 2017-01-10 13:25:59 UTC
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.
Comment 81 jmbroust 2017-03-21 13:27:13 UTC
Created attachment 61383 [details]
column more > order
Comment 82 jmbroust 2017-03-21 13:28:51 UTC
Created attachment 61384 [details]
no name when receving
Comment 83 jmbroust 2017-03-21 13:29:09 UTC
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.
Comment 84 Julian Maurice 2018-02-08 14:10:05 UTC
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.
Comment 85 Julian Maurice 2018-02-08 14:10:10 UTC
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>
Comment 86 Julian Maurice 2018-02-08 14:10:15 UTC
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>
Comment 87 Julian Maurice 2018-02-08 14:10:19 UTC
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.
Comment 88 Julian Maurice 2018-02-08 14:10:24 UTC
Created attachment 71348 [details] [review]
Bug 12395: QA follow-up - Add class/id to body tag
Comment 89 Julian Maurice 2018-02-08 14:10:29 UTC
Created attachment 71349 [details] [review]
Bug 12395: Use standard <fieldset> + <ol> markup for showorder.tt
Comment 90 Julian Maurice 2018-02-08 14:10:48 UTC
Patches rebased on master
Comment 91 Séverine Queune 2018-03-12 16:16:34 UTC
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>
Comment 92 Séverine Queune 2018-03-12 16:16:42 UTC
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>
Comment 93 Séverine Queune 2018-03-12 16:16:49 UTC
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>
Comment 94 Séverine Queune 2018-03-12 16:16:55 UTC
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>
Comment 95 Séverine Queune 2018-03-12 16:17:01 UTC
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>
Comment 96 Séverine Queune 2018-03-12 16:17:08 UTC
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>
Comment 97 Katrin Fischer 2018-04-16 06:07:23 UTC
Patch doesn't apply, please rebase and check for changes needed.
Comment 98 Julian Maurice 2018-04-16 08:22:57 UTC
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>
Comment 99 Julian Maurice 2018-04-16 08:23:04 UTC
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>
Comment 100 Julian Maurice 2018-04-16 08:23:10 UTC
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>
Comment 101 Julian Maurice 2018-04-16 08:23:16 UTC
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>
Comment 102 Julian Maurice 2018-04-16 08:23:22 UTC
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>
Comment 103 Julian Maurice 2018-04-16 08:23:28 UTC
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>
Comment 104 Julian Maurice 2018-04-16 08:24:03 UTC
Patches rebased on master
Comment 105 Katrin Fischer 2018-04-16 21:12:00 UTC
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!
Comment 106 Katrin Fischer 2018-04-16 21:12:36 UTC
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>
Comment 107 Katrin Fischer 2018-04-16 21:12:41 UTC
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>
Comment 108 Katrin Fischer 2018-04-16 21:12:46 UTC
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>
Comment 109 Katrin Fischer 2018-04-16 21:12:52 UTC
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>
Comment 110 Katrin Fischer 2018-04-16 21:12:59 UTC
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>
Comment 111 Katrin Fischer 2018-04-16 21:13:05 UTC
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>
Comment 112 Katrin Fischer 2018-04-16 21:13:11 UTC
Created attachment 74282 [details] [review]
Bug 12395: (QA follow-up) Change wording for consistency and fix use CGI;
Comment 113 Jonathan Druart 2018-04-18 20:48:17 UTC
  Bug 12395: Save order line's creator
reverts bug 20303
Comment 114 Jonathan Druart 2018-04-18 20:54:58 UTC
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
Comment 115 Julian Maurice 2018-06-08 08:26:44 UTC
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>
Comment 116 Julian Maurice 2018-06-08 08:26:51 UTC
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>
Comment 117 Julian Maurice 2018-06-08 08:26:56 UTC
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>
Comment 118 Julian Maurice 2018-06-08 08:27:03 UTC
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>
Comment 119 Julian Maurice 2018-06-08 08:27:08 UTC
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>
Comment 120 Julian Maurice 2018-06-08 08:27:14 UTC
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>
Comment 121 Julian Maurice 2018-06-08 08:27:20 UTC
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>
Comment 122 Julian Maurice 2018-06-08 08:27:26 UTC
Created attachment 75907 [details] [review]
Bug 12395: Add FK constraint and use patron-title.inc

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 123 Julian Maurice 2018-06-08 08:27:54 UTC
(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
Comment 124 Jonathan Druart 2018-06-12 15:43:20 UTC
(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.
Comment 125 Julian Maurice 2018-06-13 07:01:07 UTC
(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)
Comment 126 Jonathan Druart 2018-06-14 21:06:39 UTC
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>
Comment 127 Jonathan Druart 2018-06-14 21:06:46 UTC
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>
Comment 128 Jonathan Druart 2018-06-14 21:06:52 UTC
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>
Comment 129 Jonathan Druart 2018-06-14 21:06:59 UTC
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>
Comment 130 Jonathan Druart 2018-06-14 21:07:06 UTC
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>
Comment 131 Jonathan Druart 2018-06-14 21:07:16 UTC
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>
Comment 132 Jonathan Druart 2018-06-14 21:07:25 UTC
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>
Comment 133 Jonathan Druart 2018-06-14 21:07:33 UTC
Created attachment 76081 [details] [review]
Bug 12395: Add FK constraint and use patron-title.inc

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 134 Jonathan Druart 2018-06-14 21:10:01 UTC
(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.
Comment 135 Jonathan Druart 2018-06-14 21:10:45 UTC
Forgot the signed-off-by line, reattaching...
Comment 136 Jonathan Druart 2018-06-14 21:11:15 UTC
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>
Comment 137 Jonathan Druart 2018-06-14 21:11:23 UTC
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>
Comment 138 Jonathan Druart 2018-06-14 21:11:28 UTC
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>
Comment 139 Jonathan Druart 2018-06-14 21:11:36 UTC
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>
Comment 140 Jonathan Druart 2018-06-14 21:11:43 UTC
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>
Comment 141 Jonathan Druart 2018-06-14 21:11:50 UTC
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>
Comment 142 Jonathan Druart 2018-06-14 21:11:56 UTC
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>
Comment 143 Jonathan Druart 2018-06-14 21:12:03 UTC
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>
Comment 144 Nick Clemens 2018-07-06 14:48:08 UTC
Awesome work all!

Pushed to master for 18.11
Comment 145 Martin Renvoize 2018-07-10 08:29:05 UTC
Nice enhancement, not backporting to 18.05.x series.