|
Description
Fridolin Somers
2012-07-13 16:34:58 UTC
Created attachment 10827 [details]
New suggestion form (intranet)
Created attachment 10828 [details]
Suggestions management (intranet)
Created attachment 11100 [details] [review] Proposed patch Proposed Patch : Filter suggestions by copyrightdate instead of publicationyear (since it is copyrightdate in the suggestion creation form). Look at Suggestions Management/Bibliographic information. Corrects a bug on date filters (suggested, managed, accepted dates), only suggesteddate could work. Corrects a bug when display by itemtype : itemtype can be null or '' in db. Patch aggregates this two values into the same tab (No name). In suggestion.tt, downsizes a little bit the date fields width so that calendar icon comes on the same line (and so filters part is less high). Hi Fridolyn, You talk about copyright date...but there's no copyright date in your new form ??? You talk about itemtype...but there's non itemtype filter ?? budget and date filters dont'work. For date filters, have look to the pop-up for the calendar. There's a problem, date are translated (and searched ?) like MM/JJ/AAAA (for exemple : 01/08/2012 wrong translation in 08/01/2012) Hie Corinne. > You talk about copyright date...but there's no copyright date in your new form ??? I changed the filter on publication year into copyright date. Copyright date is suggestion form in OPAC and intranet. > You talk about itemtype...but there's non itemtype filter ?? I talk about item type in "organize by" combobox. > budget and date filters dont'work. I did not test budget filter. Maybe I'll try, otherwise I can be a new bug. > For date filters, ... Can't reproduce this problem. If it's a javascript problem, it can be a new bug. I corrected something in pl file. Best regards, Created attachment 11741 [details] [review] Proposed patch (2) Filter on budget was indeed not working. I corrected it and also removed from filters the empty values that can come form database. > budget and date filters dont'work.
Testing date filters is not easy because they don't appear in suggestions table. It will be corrected soon in master, see BZ8440.
Retest please.
Created attachment 11759 [details] [review] [SIGNED-OFF] Bug 8443: Suggestions publication year and copyright date Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested: - Copyright date is saved on creating suggestions from OPAC and staff - Copyright filter works - Form and filter labelling match - Budget filter works - When ordering an accepted suggestion the publication year is filled with the copyright date. I noted some problems I found while testing this on the bug. This patch is still an improvement to reach more consistency. Signing off. Hi Fridolyn, I found some smaller issues: 1) When I create a new order, the form says "Publication year". I think it would be good to be consistent and use "Publication year" or "Copyright date" everywhere. 2) The suggeston form in the OPAC says "Copyright Date", should be "Copyright date" according to our capitalization rules. 3) When I use filters, they get hidden again when the page reloads to show the search results. This makes it hard to see, which filters have been applied. A menu with a filter used should remain open, I think. 4) After changing the status of a suggestion, all other tabs vanish and I can only see the tab I have changed the suggestion to. And additionally, on this tab only the updated suggestion shows, even if there are more. Can we fix that? :( 5) For all to-from-filters the calendar icon gets pushed to the next line for me. That's a small thing, but would be nice if it could get fixed. 6) When you order from a suggestion, in the lists of suggestions the copyright symbol is broken. The title and year display like this: copy. year:2012 7) When I use the copyright filter the URL and the filter reflect what I did - the year still shows up when I reopen the bilbiographic filter section. When I search for a suggestion date, the date vanishes after applying the filter. And when I add another filter, all filter options are kept, except for the dates. A follow up looking into some of these problems would be great. I can also file separate bugs for those - please let me know. Thanks for testing Katrin, I think you should create new bugs to allow small patches, depending on a main bug (like 'suggestions improvements'). CONFLICT (content): Merge conflict in C4/Acquisition.pm Seems trivial. But please correct. Created attachment 12494 [details] [review] Proposed patch (rebased master) Rebased patch. Only a little conflict in a comment line in C4/Acquisition.pm QA comment: * this patch also includes a lot of indent & perltidy. *please, don't do that*, it's very hard to see what is related to your patch and what is related to perltidy. perltidy++, but in a follow-up patch please ! * passes koha-qa.pl, except for C4/Suggestions.pm FAIL pod FAIL *** WARNING: line containing nothing but whitespace in paragraph in file C4/Suggestions.pm *** ERROR: Spurious text after =cut in file C4/Suggestions.pm but it was already here before the patch passes QA Testing the feature, I've a problem: I have suggestions with copyrightdate (2011), and some without. When I filter on 2011, the suggestions without still appear. When I filter on 232545 (yes, that's not a year ;-) ), all suggestions still appear, including those 2011 as date. Could someone check the filter & tell me if it work for him/her ? Failed QA in the meantime (In reply to comment #14) > Testing the feature, I've a problem: I have suggestions with copyrightdate > (2011), and some without. > When I filter on 2011, the suggestions without still appear. > When I filter on 232545 (yes, that's not a year ;-) ), all suggestions still > appear, including those 2011 as date. > > Could someone check the filter & tell me if it work for him/her ? > > Failed QA in the meantime I can't reproduce, with copyright date 0 or NULL. Could you give a dump of your suggestions table ? (In reply to comment #15) > I can't reproduce, with copyright date 0 or NULL. > Could you give a dump of your suggestions table ? strange. I tried again, and it worked well. I set the status to passed QA. As i've declared "string freeze", I won't push this patch for 3.10.0, but it will be for 3.10.1, definetely Please do not perltidy large sections of code when fixing bugs. It makes it difficult to see what changes were made. Could you please divide this into two patches, one with the perltidy and one with the bugfix? They can be in either order (perltidy first or bugfix first). Created attachment 13265 [details] [review] Proposed patch (main) I splitted into several patches : Main patch correcting suggestions search : - allow filtering on copyrightdate, manageddate and accepteddate - entered dates remains in filter during search - empty and null values are in the same tab - null/empty values are not present in filers comboboxes Created attachment 13266 [details] [review] Proposed patch (follow-up 1) Small correction in method comments. Created attachment 13267 [details] [review] Proposed patch (follow-up 2) Cosmetic patch : Manual editions : - use of () for qw - use of q{} or qq{} in SQL queries - replacing $${key} by $->{key} - formatting SQL queries (easier to read) - in suggestion.tt reducing date input size to allow calendar icon on same line + Formatting with perltidy Needs QA again. Fridolyn, thx for splitting up the patches as requested. Can we please get a new sign-off on these patches? Have signed off, however it would have been better for the follow up 2 to have the perltidy in a separate patch. Mixing any changes with a file sized perltidy just makes testing a lot harder. Created attachment 13958 [details] [review] Bug 8443: Suggestions publication year and copyright date Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 13959 [details] [review] Bug 8443: Suggestions publication year and copyright date (follow-up 1) Comments corrections Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 13960 [details] [review] Bug 8443: Suggestions publication year and copyright date (follow-up 2) Cosmetic changes and perltidy formatting Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Working on this. Created attachment 14568 [details] [review] [SIGNED-OFF] Bug 8443: Suggestions publication year and copyright date Main patch correcting suggestions search : - allow filtering on copyrightdate, manageddate and accepteddate - entered dates remains in filter during search - empty and null values are in the same tab - null/empty values are not present in filers comboboxes Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 14569 [details] [review] Bug 8443: Suggestions publication year and copyright date (follow-up 1) Comments corrections Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 14570 [details] [review] [SIGNED-OFF] Bug 8443: Suggestions publication year and copyright date (follow-up 2) Cosmetic changes and perltidy formatting Cosmetic patch : Manual editions : - use of () for qw - use of q{} or qq{} in SQL queries - replacing $${key} by $->{key} - formatting SQL queries (easier to read) - in suggestion.tt reducing date input size to allow calendar icon on same line + Formatting with perltidy Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests were done with all 3 patches from this bug applied: 1) Add suggestion from OPAC, check copyright date is saved 2) Add suggestion in intranet, check copyright date is saved 3) Edit suggestion, check change is saved correctly 4) Check date filters work as expected, using single dates, date ranges and multiple filters at once: - Accepted on - Managed on - Suggested on 5) Check copyright date search works correctly. Passes all tests and QA script. This patch has been pushed to master. |