Bug 13392 - New statistics wizard for suggestions
Summary: New statistics wizard for suggestions
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-04 14:53 UTC by Julian Maurice
Modified: 2017-06-20 20:44 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.45 KB, patch)
2014-12-04 14:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.70 KB, patch)
2014-12-04 14:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.70 KB, patch)
2014-12-05 09:15 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.64 KB, patch)
2014-12-05 09:19 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.48 KB, patch)
2014-12-11 15:48 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.70 KB, patch)
2014-12-11 15:48 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.76 KB, patch)
2015-02-11 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.64 KB, patch)
2015-02-11 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.75 KB, patch)
2015-02-11 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.85 KB, patch)
2015-02-26 15:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.64 KB, patch)
2015-02-26 15:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.75 KB, patch)
2015-02-26 15:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.91 KB, patch)
2015-03-02 16:11 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.71 KB, patch)
2015-03-02 16:11 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.82 KB, patch)
2015-03-02 16:11 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.58 KB, patch)
2015-06-18 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.69 KB, patch)
2015-06-18 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.90 KB, patch)
2015-06-18 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review
[Follow-up] Bug 13392 - Fix QA failures: (2.94 KB, patch)
2015-06-18 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.58 KB, patch)
2016-02-22 12:51 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (23.73 KB, patch)
2016-02-22 12:51 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.89 KB, patch)
2016-02-22 12:51 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392 - Fix QA failures: (2.93 KB, patch)
2016-02-22 12:51 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes (1.95 KB, patch)
2016-03-14 10:56 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.58 KB, patch)
2016-06-03 12:30 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (24.00 KB, patch)
2016-06-03 12:30 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.88 KB, patch)
2016-06-03 12:30 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Fix QA failures: (2.93 KB, patch)
2016-06-03 12:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes (1.95 KB, patch)
2016-06-03 12:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: Remove use of C4::Branch and C4::Category (2.67 KB, patch)
2016-10-10 14:09 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Follow-up on terminology (1.24 KB, patch)
2016-10-26 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.61 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: New statistics assistant for suggestions (24.05 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: QA fixes (8.92 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Fix QA failures: (2.94 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes (1.96 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392: Remove use of C4::Branch and C4::Category (2.68 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13392 - Follow-up on terminology (1.25 KB, patch)
2017-03-09 14:35 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392: Add suggestions.ordereddate and suggestions.orderedby (5.65 KB, patch)
2017-06-20 17:57 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392: New statistics assistant for suggestions (24.07 KB, patch)
2017-06-20 17:57 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392: QA fixes (8.95 KB, patch)
2017-06-20 17:57 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392 - Fix QA failures: (2.99 KB, patch)
2017-06-20 17:57 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes (2.02 KB, patch)
2017-06-20 17:57 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392: Remove use of C4::Branch and C4::Category (2.74 KB, patch)
2017-06-20 17:58 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13392 - Follow-up on terminology (1.31 KB, patch)
2017-06-20 17:58 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2014-12-04 14:53:13 UTC
Statistics wizard for suggestions
Comment 1 Julian Maurice 2014-12-04 14:55:08 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2014-12-04 14:55:11 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2014-12-04 15:00:27 UTC
Test plan:
1/ Go to Reports > Statistics wizard > Suggestions
2/ Play with it and check if result is consistent with what you have in your database.

All fields used in this wizard correspond to a column in the suggestions table, except for categorycode, sort1 and sort2, which correspond to the borrowers fields of the user who made the suggestion (suggestedby).
Comment 4 Julian Maurice 2014-12-05 09:15:10 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2014-12-05 09:19:05 UTC Comment hidden (obsolete)
Comment 6 beroud 2014-12-11 15:38:34 UTC
All fields used in this wizard correspond to a column in the suggestions table : it's ok.
Comment 7 sandboxes@biblibre.com 2014-12-11 15:47:38 UTC
Patch tested with a sandbox, by Beroud <isabelle.beroud@univ-lyon3.fr>
Comment 8 sandboxes@biblibre.com 2014-12-11 15:48:23 UTC Comment hidden (obsolete)
Comment 9 sandboxes@biblibre.com 2014-12-11 15:48:26 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2015-01-06 01:21:46 UTC
Hi Julian,

I feel like this would have been a bit better split up into 2 separate bugs/features. Tried a few things and have some first feedback from code review and tests:

Bug 13392: Add suggestions.ordereddate and suggestions.orderedby 

1) I am not sure about adding ordereddate and orderedby as those seem duplicating the information we can get from aqorders.entrydate and aqbasket.authorisedby.


Bug 13392: New statistics assistant for suggestions

1) The new tests don't give any information when run, please add a description.

2) Using the Koha:I18N is an option now, but I think we haven't created the po file that is needed for it yet. Those strings appear already multiple times in the templates, so the translators have to make sure to translate them the same as everywhere else. I feel it might be easier if we moved them to the template. But more important, the custom suggestion statuses are not supported by the new wizard (authorized value SUGGEST_STATUS)

3) For MARC21 the publicationyear is saved in the copyrightdate column in the database. Please change so that copyrightdate is used in the SQL query for MARC21 databases.

4) The bsort1 and bsort2 fields can be freetext if there are no entries in the authorised value categories - maybe show an input field in that case?

5) Please rename branch to library on the suggestion form. And maybe just "Reason" instead of "Reject reason"? The reasons can also be positive.

In testing this I had difficulties with getting a suggestion into the ORDERED status with and without the patches - can you check if that works for you? The new database fields stayed empty too :(
Comment 11 Julian Maurice 2015-02-11 12:40:03 UTC
> 1) I am not sure about adding ordereddate and orderedby as those seem
> duplicating the information we can get from aqorders.entrydate and
> aqbasket.authorisedby.

It seems there is no direct link between a suggestion and an order line.
Are you suggesting we should create this link ?


> 3) For MARC21 the publicationyear is saved in the copyrightdate column
> in the database. Please change so that copyrightdate is used in the
> SQL query for MARC21 databases.

It seems in fact that suggestions.publicationyear is not used at all (my
own database only contains NULL or 0 in this column whereas
copyrightdate contains the actual data)


> In testing this I had difficulties with getting a suggestion into the
> ORDERED status with and without the patches - can you check if that
> works for you? The new database fields stayed empty too :(

Yes, it worked for me, but I will re-check.
Comment 12 Julian Maurice 2015-02-11 14:35:32 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2015-02-11 14:35:45 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2015-02-11 14:35:52 UTC Comment hidden (obsolete)
Comment 15 Julian Maurice 2015-02-11 14:40:46 UTC
> > In testing this I had difficulties with getting a suggestion into
> > the ORDERED status with and without the patches - can you check if
> > that works for you? The new database fields stayed empty too :(
> 
> Yes, it worked for me, but I will re-check.

Apparently it works only when the suggestion is not linked to an existing record (ie. the record is created while creating the order line). I don't know if this is a bug or not.

And of course you need to run updatedatabase and misc/devel/update_dbix_class_files.pl (I always forget the latter)
Comment 16 Katrin Fischer 2015-02-11 21:25:10 UTC
Hi Julian,

sorry, I don't understand the last comment - can you give an example/step by step for both cases maybe? Thx!
Comment 17 Julian Maurice 2015-02-12 08:23:03 UTC
Here's a simple step by step example that worked for me:

1/ Create a new suggestion
2/ Mark it as ACCEPTED
3/ Create a new order from this suggestion

After these steps a new biblio record should be created, the suggestion should be linked to it and its status should be ORDERED.
In the basket view, you should see a link to the suggestion.
But before that make sure you've run the updatedatabase script and the script in misc/devel/update_dbix_class_files.pl

My remark was about if we create a suggestion from a suggestion that is already linked to a biblionumber (maybe the suggestion was already ORDERED and someone reset status to ACCEPTED), then the suggestion status is not updated.
Comment 18 Jonathan Druart 2015-02-26 15:13:10 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2015-02-26 15:13:14 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2015-02-26 15:13:20 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-02-26 15:13:51 UTC
Last patches fixed an easy conflict on updatedatabase.pl
Comment 22 sandboxes@biblibre.com 2015-03-02 16:11:14 UTC
Patch tested with a sandbox, by Nicole Engard <nengard@bywatersolutions.com>
Comment 23 sandboxes@biblibre.com 2015-03-02 16:11:51 UTC Comment hidden (obsolete)
Comment 24 sandboxes@biblibre.com 2015-03-02 16:11:54 UTC Comment hidden (obsolete)
Comment 25 sandboxes@biblibre.com 2015-03-02 16:11:58 UTC Comment hidden (obsolete)
Comment 26 Courret 2015-03-02 16:42:13 UTC
The suggest date wizard works good. 

But it seems that Koha reports some suggestions that were rejected, even if these suggestions do not appear in the staff interface.  

Some rejected suggestions that were refused for a lack of budget in the pro interface do not appear later in the wizard report. 

Similar issue of inconsistency of results for pending suggestions : 9 reported in the wizard while 19 appear in Koha pro interface. 

Unfortunately, we miss a running phpmyadmin interface to check the real content of the database.
Comment 27 Katrin Fischer 2015-04-06 11:43:26 UTC
Hi Courret, you could create a sql report with the following statement:

select * from suggestions;

This will show you everything that is in your suggestions table.
Comment 28 Katrin Fischer 2015-04-06 12:02:53 UTC
Hi Julian,

working on this again:

1) The QA script outputs some warnings, could you please take a look? I can also see those in my logs on accessing the new page:
Processing files before patches
|========================>| 13 / 13 (100.00%)
Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
Argument "" isn't numeric in numeric gt (>) at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/reports/suggestions_stats.tt line 52.
|========================>| 13 / 13 (100.00%)

2) Some strings need to be changed from 'Acquisitions' to 'Suggestions' - like the table legend on the form and the heading on the result page.

3) We need some POD for the new GetBudgetDescendantsIds.

4) I was worried about some of the TT constructs translatability and ran some tests, but looking good so far :)
Comment 29 Alex Arnaud 2015-06-18 14:28:31 UTC Comment hidden (obsolete)
Comment 30 Alex Arnaud 2015-06-18 14:28:36 UTC Comment hidden (obsolete)
Comment 31 Alex Arnaud 2015-06-18 14:28:41 UTC Comment hidden (obsolete)
Comment 32 Alex Arnaud 2015-06-18 14:28:45 UTC Comment hidden (obsolete)
Comment 33 Alex Arnaud 2015-06-18 14:30:30 UTC
I rebased patches on master / updated them to use atomic update and fix Katrin' QA comments.
Comment 34 Bernardo Gonzalez Kriegel 2015-12-18 14:04:16 UTC
Sorry, does not apply

CONFLICT (content): Merge conflict in t/db_dependent/Budgets.t
Auto-merging C4/Budgets.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 13392: New statistics assistant for suggestions
Comment 35 Alex Arnaud 2016-02-22 12:51:12 UTC Comment hidden (obsolete)
Comment 36 Alex Arnaud 2016-02-22 12:51:21 UTC Comment hidden (obsolete)
Comment 37 Alex Arnaud 2016-02-22 12:51:31 UTC Comment hidden (obsolete)
Comment 38 Alex Arnaud 2016-02-22 12:51:38 UTC Comment hidden (obsolete)
Comment 39 Alex Arnaud 2016-03-14 10:56:01 UTC Comment hidden (obsolete)
Comment 40 Julian Maurice 2016-06-03 12:30:48 UTC
Created attachment 51995 [details] [review]
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby

Those fields are updated when suggestion status is changed to 'ORDERED'
and are displayed on suggestion/suggestion.pl page

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Patch updated: Use atomic update
Comment 41 Julian Maurice 2016-06-03 12:30:52 UTC
Created attachment 51996 [details] [review]
Bug 13392: New statistics assistant for suggestions

New sub C4::Budgets::GetBudgetDescendantsIds (with unit tests)

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 42 Julian Maurice 2016-06-03 12:30:57 UTC
Created attachment 51997 [details] [review]
Bug 13392: QA fixes

1/ Add tests descriptions
2/ Do not use Koha::I18N for suggestion statuses, and add statuses from
SUGGEST_STATUS authorised values
3/ Use copyrightdate instead of publicationyear which seems to be unused
4/ Display free text filters if there is no values in corresponding
authorised values list
5/ Rename 'Branch' to 'Library' and 'Reject reason' to 'Reason'

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 43 Julian Maurice 2016-06-03 12:31:01 UTC
Created attachment 51998 [details] [review]
Bug 13392 - Fix QA failures:

  - Fix a perl warning in suggestions_stats.tt,
  - Replace term "Acquisitions" with "Suggestions",
  - Add POD for GetBudgetDescendantsIds.
Comment 44 Julian Maurice 2016-06-03 12:31:05 UTC
Created attachment 51999 [details] [review]
Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes
Comment 45 Julian Maurice 2016-06-03 12:31:52 UTC
Patches rebased on master
Comment 46 François Pichenot 2016-10-10 09:52:36 UTC
This patch doesn't apply, I get this message :

Can't locate C4/Branch.pm in @INC (@INC contains: /home/sandbox1/src /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl .) at /home/sandbox1/src/reports/suggestions_stats.pl line 27.
BEGIN failed--compilation aborted at /home/sandbox1/src/reports/suggestions_stats.pl line 27.
Comment 47 Julian Maurice 2016-10-10 14:09:56 UTC
Created attachment 56110 [details] [review]
Bug 13392: Remove use of C4::Branch and C4::Category

+ fix a 'ambiguous column name' error when using borrowers table
Comment 48 Katrin Fischer 2016-10-26 21:53:56 UTC
Created attachment 56889 [details] [review]
Bug 13392 - Follow-up on terminology

Standardizing a few terms
- Patron sort 1/2 to match the holds statistics wizard
- Patron category instead of borrower category
Comment 49 Katrin Fischer 2016-10-26 21:58:31 UTC
Hi Julian,
I wrote a tiny follow up changing some of the terms used - can you please check?
I concentrated on the changes/new columns in suggestions to see if this works alright. While testing I found that when ordering from a suggestion the status did no longer switch to ORDERED and the suggestion remains as ACCEPTED. Please fix!
Comment 50 Julian Maurice 2017-03-09 14:35:27 UTC
Created attachment 60956 [details] [review]
Bug 13392: Add suggestions.ordereddate and suggestions.orderedby

Those fields are updated when suggestion status is changed to 'ORDERED'
and are displayed on suggestion/suggestion.pl page

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Patch updated: Use atomic update
Comment 51 Julian Maurice 2017-03-09 14:35:32 UTC
Created attachment 60957 [details] [review]
Bug 13392: New statistics assistant for suggestions

New sub C4::Budgets::GetBudgetDescendantsIds (with unit tests)

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 52 Julian Maurice 2017-03-09 14:35:37 UTC
Created attachment 60958 [details] [review]
Bug 13392: QA fixes

1/ Add tests descriptions
2/ Do not use Koha::I18N for suggestion statuses, and add statuses from
SUGGEST_STATUS authorised values
3/ Use copyrightdate instead of publicationyear which seems to be unused
4/ Display free text filters if there is no values in corresponding
authorised values list
5/ Rename 'Branch' to 'Library' and 'Reject reason' to 'Reason'

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 53 Julian Maurice 2017-03-09 14:35:42 UTC
Created attachment 60959 [details] [review]
Bug 13392 - Fix QA failures:

  - Fix a perl warning in suggestions_stats.tt,
  - Replace term "Acquisitions" with "Suggestions",
  - Add POD for GetBudgetDescendantsIds.
Comment 54 Julian Maurice 2017-03-09 14:35:47 UTC
Created attachment 60960 [details] [review]
Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes
Comment 55 Julian Maurice 2017-03-09 14:35:51 UTC
Created attachment 60961 [details] [review]
Bug 13392: Remove use of C4::Branch and C4::Category

+ fix a 'ambiguous column name' error when using borrowers table
Comment 56 Julian Maurice 2017-03-09 14:35:56 UTC
Created attachment 60962 [details] [review]
Bug 13392 - Follow-up on terminology

Standardizing a few terms
- Patron sort 1/2 to match the holds statistics wizard
- Patron category instead of borrower category
Comment 57 Julian Maurice 2017-03-09 14:36:27 UTC
Patches rebased on master
Comment 58 Julian Maurice 2017-03-09 14:39:45 UTC
Katrin, sorry but I wasn't able to reproduce your problem. When I create a new order line from a suggestion, the suggestion status is always changed to ORDERED.
Could you describe step by step what you do ?
Comment 59 Katrin Fischer 2017-05-02 21:48:52 UTC
I've tried testing this by applying the patches, running the database update, regenerating the Schema. Every time I try to order from my accepted suggestion, an internal server error is shown (running under Plack with kohadevbox). 
Without Plack the error shown is:
Template process failed: undef error - The method selected is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

This prevents me from testing the problem I had earlier :( Filed bug 18525 as I also see this without the patches. But would be good if someone could confirm.
Comment 60 Josef Moravec 2017-06-20 17:57:37 UTC
Created attachment 64459 [details] [review]
[SIGNED-OFF] Bug 13392: Add suggestions.ordereddate and suggestions.orderedby

Those fields are updated when suggestion status is changed to 'ORDERED'
and are displayed on suggestion/suggestion.pl page

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Patch updated: Use atomic update

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 61 Josef Moravec 2017-06-20 17:57:42 UTC
Created attachment 64460 [details] [review]
[SIGNED-OFF] Bug 13392: New statistics assistant for suggestions

New sub C4::Budgets::GetBudgetDescendantsIds (with unit tests)

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 62 Josef Moravec 2017-06-20 17:57:47 UTC
Created attachment 64461 [details] [review]
[SIGNED-OFF] Bug 13392: QA fixes

1/ Add tests descriptions
2/ Do not use Koha::I18N for suggestion statuses, and add statuses from
SUGGEST_STATUS authorised values
3/ Use copyrightdate instead of publicationyear which seems to be unused
4/ Display free text filters if there is no values in corresponding
authorised values list
5/ Rename 'Branch' to 'Library' and 'Reject reason' to 'Reason'

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 63 Josef Moravec 2017-06-20 17:57:52 UTC
Created attachment 64462 [details] [review]
[SIGNED-OFF] Bug 13392 - Fix QA failures:

  - Fix a perl warning in suggestions_stats.tt,
  - Replace term "Acquisitions" with "Suggestions",
  - Add POD for GetBudgetDescendantsIds.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 64 Josef Moravec 2017-06-20 17:57:56 UTC
Created attachment 64463 [details] [review]
[SIGNED-OFF] Bug 13392 - Replace using of C4::Dates and C4::ItemType by Koha::DateUtils and Koha::ItemTypes

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 65 Josef Moravec 2017-06-20 17:58:00 UTC
Created attachment 64464 [details] [review]
[SIGNED-OFF] Bug 13392: Remove use of C4::Branch and C4::Category

+ fix a 'ambiguous column name' error when using borrowers table

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 66 Josef Moravec 2017-06-20 17:58:05 UTC
Created attachment 64465 [details] [review]
[SIGNED-OFF] Bug 13392 - Follow-up on terminology

Standardizing a few terms
- Patron sort 1/2 to match the holds statistics wizard
- Patron category instead of borrower category

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 67 Josef Moravec 2017-06-20 17:58:41 UTC
(In reply to Katrin Fischer from comment #59)
> I've tried testing this by applying the patches, running the database
> update, regenerating the Schema. Every time I try to order from my accepted
> suggestion, an internal server error is shown (running under Plack with
> kohadevbox). 
> Without Plack the error shown is:
> Template process failed: undef error - The method selected is not covered by
> tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.
> 
> This prevents me from testing the problem I had earlier :( Filed bug 18525
> as I also see this without the patches. But would be good if someone could
> confirm.

I can't confirm it, it does work for me...
Comment 68 Katrin Fischer 2017-06-20 20:44:32 UTC
The blocking bug was fixed since, but I didn't get back to this - thanks for testing Josef!