Bug 5357 - Subscription search and displays
Summary: Subscription search and displays
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Deadline: 2011-05-01
Assignee: Jonathan Druart
QA Contact: Mason James
URL: http://wiki.koha-community.org/wiki/S...
Keywords:
Depends on: 5337
Blocks: 8415 8782
  Show dependency treegraph
 
Reported: 2010-11-02 03:53 UTC by Paul Poulain
Modified: 2013-05-23 06:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 5357: Adds a new page for searching subscriptions (28.21 KB, patch)
2012-06-22 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5357: Adds a new page for searching subscriptions (28.27 KB, patch)
2012-08-01 14:47 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 8443: Suggestions publication year and copyright date (10.88 KB, patch)
2012-08-01 14:48 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 8443: Suggestions publication year and copyright date (10.93 KB, patch)
2012-08-01 14:49 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 8443: Suggestions publication year and copyright date (10.93 KB, patch)
2012-08-01 14:50 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 5357: Adds a new page for searching subscriptions (29.85 KB, patch)
2012-08-02 08:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5357: Adds a new page for searching subscriptions (29.90 KB, patch)
2012-08-02 09:57 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 5357: Adds a new page for searching subscriptions (30.88 KB, patch)
2012-08-22 07:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5357: Adds a new page for searching subscriptions (30.95 KB, patch)
2012-08-25 21:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5357: Follow-up - layout and EAN search (3.01 KB, patch)
2012-08-25 21:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5357: Follow-up - layout and EAN search (3.09 KB, patch)
2012-08-27 08:39 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 Paul Poulain 2010-11-02 03:53:44 UTC
* The search page will be improved and some fields will be added to search on (issn, subscription number, library, internal note, callnumber)
* The subscription list result will be modified. The " will be removed in case of a repeated title & issn (the title & issn will be repeated)
* The internal note is not displayed on staff interface. It will be displayed in serial issues and in various places where it can be usefull
* An option will be added to limit subscription search by branch
Comment 1 Jonathan Druart 2012-06-22 14:15:42 UTC Comment hidden (obsolete)
Comment 2 Corinne HAYET 2012-08-01 14:41:15 UTC
ISSN and Bookseller search don't work
Comment 3 Paul Poulain 2012-08-01 14:47:11 UTC
Patch tested with a sandbox, by Corinne HAYET <corinne.hayet@bulac.fr>
Comment 4 Paul Poulain 2012-08-01 14:47:31 UTC Comment hidden (obsolete)
Comment 5 Paul Poulain 2012-08-01 14:48:11 UTC
Patch tested with a sandbox, by Corinne HAYET <corinne.hayet@bulac.fr>
Comment 6 Paul Poulain 2012-08-01 14:48:32 UTC Comment hidden (obsolete)
Comment 7 Paul Poulain 2012-08-01 14:49:12 UTC
Patch tested with a sandbox, by Corinne HAYET <corinne.hayet@bulac.fr>
Comment 8 Paul Poulain 2012-08-01 14:49:29 UTC Comment hidden (obsolete)
Comment 9 Paul Poulain 2012-08-01 14:50:10 UTC
Patch tested with a sandbox, by Corinne HAYET <corinne.hayet@bulac.fr>
Comment 10 Paul Poulain 2012-08-01 14:50:29 UTC Comment hidden (obsolete)
Comment 11 Paul Poulain 2012-08-01 14:53:50 UTC
Comment on attachment 11263 [details] [review]
Bug 5357: Adds a new page for searching subscriptions

unobsoleting this patch, signed-off by Corinne Hayet on camembert sandbox

Will obsolete the patch not related to this bug, I made a mistake on my sandbox setup, he made silly things. Should be fixed now
Comment 12 Katrin Fischer 2012-08-01 15:08:50 UTC
Hi Corinne, 
was your sign-off on this intended? I see you posted a comment first:
"ISSN and Bookseller search don't work"
Can you confirm all is ok? Thank you and thanks for testing!
Comment 13 Corinne HAYET 2012-08-01 15:27:24 UTC
(En réponse au commentaire 12)
> Hi Corinne, 
> was your sign-off on this intended? I see you posted a comment first:
> "ISSN and Bookseller search don't work"
> Can you confirm all is ok? Thank you and thanks for testing!

Hi Katrin,

No, it's a mistake :(

This patch is not sign-off because ISSN and Bookseller search don't work
Comment 14 Paul Poulain 2012-08-01 15:34:40 UTC
so, failing QA

(Corinne now knows when to use the 2nd part of the sandbox = only if you want to signoff !)
Comment 15 Jonathan Druart 2012-08-02 08:41:17 UTC Comment hidden (obsolete)
Comment 16 Paul Poulain 2012-08-02 09:57:11 UTC
Patch tested with a sandbox, by Corinne HAYET <corinne.hayet@bulac.fr>
Comment 17 Paul Poulain 2012-08-02 09:57:30 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2012-08-22 06:24:52 UTC
Hi, I tried giving this another sign-off, but the patch no longer applies. I tried fixing the conflict, but I am not sure how to resolve it. There are recent changes to serials-home that might get undone by adding this patch and moving things into includes (see bug 8432). As this is blocking bug 8415 it would be nice if somoene could take a look.
Comment 19 Jonathan Druart 2012-08-22 07:19:59 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2012-08-25 21:33:02 UTC
Created attachment 11831 [details] [review]
[SIGNED-OFF] Bug 5357: Adds a new page for searching subscriptions

Test plan:
- go on the serial module
- click on the 'Advanced search' link (right of subscriptions search in
the header)
- Search subscriptions (by ISSN, title, EAN, Publisher, Supplier and/or
  Branch)
- Check results are correct

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 21 Katrin Fischer 2012-08-25 21:33:10 UTC Comment hidden (obsolete)
Comment 22 Katrin Fischer 2012-08-25 21:37:27 UTC
I noticed that the EAN search option was not hidden for MARC21 and some differences in the layout of the form compared to the advance search for acquisitions.

I wrote a follow up patch - Jonathan, maybe you can take a look and sign off if it's ok for you?

I noticed that the new serials-search.tt file is using 2 spaces for identation. Maybe we should fix that too?
Comment 23 Jonathan Druart 2012-08-27 08:39:35 UTC
Created attachment 11839 [details] [review]
Bug 5357: Follow-up - layout and EAN search

1) EAN search should be hidden if marcflavour is not UNIMARC
2) Fixes layout to match the advanced search in acquisitions
- labels in front of the fields instead of separate lines
- adds a legend to the form and moves the toggle for the search form to it

To test:
- EAN search field should only show up when marcflavour system preference
  is set to UNIMARC
- Check layout is consistent and you like it
- Check toggle for advanced search still behaves the same

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 24 Jonathan Druart 2012-08-27 08:53:40 UTC
(In reply to comment #22)
> I noticed that the new serials-search.tt file is using 2 spaces for
> identation. Maybe we should fix that too?

Hi Katrin,
I think there is no coding guidelines for that in the wiki page.
For the tt files, if we choose 4 spaces, after the 4 mandatory div (<div id="doc3" class="yui-t2"><div id="bd"><div id="yui-main"><div class="yui-b">), we write each line with 12 spaces. After 3 or 4 blocks, we have a indentation of 24-28 spaces. It's why I prefer to choose only 2 spaces.
It could be possible to not indent the 4 first div, but the readabillity would be bad.
I am open-minded on this subject especially if a consensus already exists.
Comment 25 Mason James 2012-09-16 14:03:09 UTC
(In reply to comment #24)
> (In reply to comment #22)
> > I noticed that the new serials-search.tt file is using 2 spaces for
> > identation. Maybe we should fix that too?
> 
> Hi Katrin,
> I think there is no coding guidelines for that in the wiki page.
> For the tt files, if we choose 4 spaces, after the 4 mandatory div (<div
> id="doc3" class="yui-t2"><div id="bd"><div id="yui-main"><div
> class="yui-b">), we write each line with 12 spaces. After 3 or 4 blocks, we
> have a indentation of 24-28 spaces. It's why I prefer to choose only 2
> spaces.

yes, i completely agree Jonathan! :)

lets remember to discuss this sometime on the koha-lists
Comment 26 Mason James 2012-09-16 14:14:26 UTC
> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

patches looks good, passing QA…

$ koha-qa.pl  -c 2 

testing 2 commit(s) (applied to commit 2a60d49)
 * 02e2c00 Bug 5357: Follow-up - layout and EAN search
      koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
      serials/serials-search.pl

 * 3f31617 Bug 5357: Adds a new page for searching subscriptions
      C4/Serials.pm
      koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc
      koha-tmpl/intranet-tmpl/prog/en/includes/serials-search.inc
      koha-tmpl/intranet-tmpl/prog/en/modules/serials/serial-issues.tt
      koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt
      koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
      serials/serials-home.pl
      serials/serials-search.pl

* serials/serials-search.pl                                                OK
* C4/Serials.pm                                                            OK
* serials/serials-home.pl                                                  OK
* koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt        OK
* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc            OK
* koha-tmpl/intranet-tmpl/prog/en/includes/serials-search.inc              OK
* koha-tmpl/intranet-tmpl/prog/en/modules/serials/serial-issues.tt         OK
* koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt          OK
Comment 27 Paul Poulain 2012-09-18 20:39:39 UTC
Patch pushed to master