Bug 20941 - Displaying requests does not display request material type
Summary: Displaying requests does not display request material type
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Andrew Isherwood
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 18591
Blocks: 20548
  Show dependency treegraph
 
Reported: 2018-06-14 13:02 UTC by Andrew Isherwood
Modified: 2019-10-14 19:58 UTC (History)
6 users (show)

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


Attachments
Bug 20941: Add Illrequest->getType method (4.66 KB, patch)
2018-06-14 13:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add Illrequest->getType method (4.71 KB, patch)
2018-09-28 12:53 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20941: Add unit tests (1.33 KB, patch)
2018-10-02 14:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (follow-up) Carry out comment #4 advice (6.43 KB, patch)
2018-10-03 11:08 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add Illrequest->getType method (4.84 KB, patch)
2018-10-04 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: Add unit tests (1.38 KB, patch)
2018-10-04 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Carry out comment #4 advice (6.46 KB, patch)
2018-10-04 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant (4.53 KB, patch)
2018-10-04 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Wrap N/A strings in <span> (5.90 KB, patch)
2018-10-04 12:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (follow-up) Update N/A handling (6.63 KB, patch)
2018-10-05 08:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add Illrequest->getType method (4.86 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add unit tests (1.38 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (follow-up) Carry out comment #4 advice (6.48 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant (4.56 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (follow-up) Wrap N/A strings in <span> (5.90 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: (follow-up) Update N/A handling (6.63 KB, patch)
2018-10-05 14:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add Illrequest->getType method (4.84 KB, patch)
2018-10-05 15:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: Add unit tests (1.38 KB, patch)
2018-10-05 15:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: (follow-up) Carry out comment #4 advice (6.51 KB, patch)
2018-10-05 15:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant (4.54 KB, patch)
2018-10-05 15:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: (follow-up) Wrap N/A strings in <span> (3.25 KB, patch)
2018-10-05 15:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: (follow-up) Update N/A handling (3.58 KB, patch)
2018-10-05 15:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20941: (follow-up) Switch 'media' -> 'types' (1.82 KB, patch)
2018-10-08 12:42 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20941: Add Illrequest->getType method (4.95 KB, patch)
2018-10-31 09:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: Add unit tests (1.38 KB, patch)
2018-10-31 09:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Carry out comment #4 advice (6.72 KB, patch)
2018-10-31 09:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant (4.64 KB, patch)
2018-10-31 09:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Wrap N/A strings in <span> (3.41 KB, patch)
2018-10-31 09:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Update N/A handling (3.74 KB, patch)
2018-10-31 09:54 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20941: (follow-up) Switch 'media' -> 'types' (1.86 KB, patch)
2018-10-31 09:54 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 Andrew Isherwood 2018-06-14 13:02:20 UTC
I'm not sure when this crept in. At some point we switched from storing material type in illrequests.medium to an arbitrary attribute in illrequestattributes. However, the display end of things wasn't updated accordingly.

As such, material type doesn't display in any of the UIs, it should!
Comment 1 Andrew Isherwood 2018-06-14 13:32:56 UTC
Created attachment 76052 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated
Comment 2 PTFS Europe Sandboxes 2018-09-28 12:53:14 UTC
Created attachment 79577 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 3 Andrew Isherwood 2018-10-02 14:51:31 UTC
Created attachment 79825 [details] [review]
Bug 20941: Add unit tests
Comment 4 Josef Moravec 2018-10-02 15:47:48 UTC
Comment on attachment 79577 [details] [review]
Bug 20941: Add Illrequest->getType method

Review of attachment 79577 [details] [review]:
-----------------------------------------------------------------

::: Koha/Illrequest.pm
@@ +724,4 @@
>          || "";                  # "the empty prefix"
>  }
>  
> +=head3 getType

should be get_type, or maybe better get_material_type, please rename the method

@@ +734,5 @@
> +
> +sub getType {
> +    my ($self) = @_;
> +    my $attr = $self->illrequestattributes->find({ type => 'type'});
> +    return $attr ? $attr->value : '<span>N/A</span>';

We should return implicit undef if no type is set and then handle it in templates.
Comment 5 Andrew Isherwood 2018-10-03 11:08:02 UTC
Created attachment 79865 [details] [review]
Bug 20941: (follow-up) Carry out comment #4 advice

- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template
Comment 6 Andrew Isherwood 2018-10-03 11:19:58 UTC
Thanks for the review Joseph, all great points, much appreciated.
Comment 7 Andrew Isherwood 2018-10-03 16:01:17 UTC
s/ph$/f/ (sorry!)
Comment 8 Josef Moravec 2018-10-04 07:51:36 UTC
Created attachment 79918 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2018-10-04 07:51:39 UTC
Created attachment 79919 [details] [review]
Bug 20941: Add unit tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Josef Moravec 2018-10-04 07:51:43 UTC
Created attachment 79920 [details] [review]
Bug 20941: (follow-up) Carry out comment #4 advice

- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2018-10-04 07:51:46 UTC
Created attachment 79921 [details] [review]
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2018-10-04 07:52:10 UTC
I added a follow-up:
 - in get_type the return could be explicit undef, which is wrong
 - the test needed to adjust to test for undef when no type is set
 - in templates I changed ?: to || for consistancy
 - in opac templates I removed span tag

I've also added filters during rebase
Comment 13 Andrew Isherwood 2018-10-04 08:38:06 UTC
Thanks for that Josef, much better! The inclusion of the span was based on advice I've been given in the past that wrapping strings with a span aids with translation, though I'm not clear how. Anyway, many thanks for your work on this, it's much appreciated!
Comment 14 Josef Moravec 2018-10-04 12:04:10 UTC
(In reply to Andrew Isherwood from comment #13)
> Thanks for that Josef, much better! The inclusion of the span was based on
> advice I've been given in the past that wrapping strings with a span aids
> with translation, though I'm not clear how. Anyway, many thanks for your
> work on this, it's much appreciated!

Yes it is right, I saw you done that on other patches... so maybe we should do it here right now...
Comment 15 Andrew Isherwood 2018-10-04 12:40:17 UTC
Created attachment 79945 [details] [review]
Bug 20941: (follow-up) Wrap N/A strings in <span>

All N/A strings in ILL templates wrapped in <span> to aid translation
Comment 16 Andrew Isherwood 2018-10-04 12:41:20 UTC
Hi Josef, I've now wrapped all N/A in <span> tags, so this should be correct and consistent now. Many thanks.
Comment 17 Katrin Fischer 2018-10-04 20:13:15 UTC
It would be good to test that this construct actually gets picked up. I am not sure if our script look for HTML inside the TT constructs... I think it would be safer not to take shortcuts here:

[% request.cost || '<span>N/A</span>' | html %]

Could also be:

[% IF ( request.cost ) %][%request.cost%][% ELSE %]<span>N/A</span>[% END %]

To test, in koha-shell you can do:

cd misc/translator
perl translate update de-DE (or any other language)

Then check the updated po files for N/A.
Comment 18 Katrin Fischer 2018-10-04 20:13:47 UTC
... also... should the html filter not escape that?
Comment 19 Katrin Fischer 2018-10-04 20:15:28 UTC
[% request.cost | html %] of course (update your QA script!)
Comment 20 Josef Moravec 2018-10-05 05:26:48 UTC
(In reply to Katrin Fischer from comment #17)
> It would be good to test that this construct actually gets picked up. I am
> not sure if our script look for HTML inside the TT constructs... I think it
> would be safer not to take shortcuts here:
> 
> [% request.cost || '<span>N/A</span>' | html %]

True, this also has problem, that html tags are escaped an showed to user...

> 
> Could also be:
> 
> [% IF ( request.cost ) %][%request.cost%][% ELSE %]<span>N/A</span>[% END %]

Thanks Katrin, that's the way it should be done...

> To test, in koha-shell you can do:
> 
> cd misc/translator
> perl translate update de-DE (or any other language)
> 
> Then check the updated po files for N/A.
Comment 21 Andrew Isherwood 2018-10-05 07:49:11 UTC
Thanks very much for that Katrin. I'll create a follow up for these today.
Comment 22 Andrew Isherwood 2018-10-05 08:02:27 UTC
Created attachment 79999 [details] [review]
Bug 20941: (follow-up) Update N/A handling

Modify the conditional display of N/A in accordance with comment #17
Comment 23 Andrew Isherwood 2018-10-05 14:27:05 UTC
Created attachment 80112 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 24 Andrew Isherwood 2018-10-05 14:27:09 UTC
Created attachment 80113 [details] [review]
Bug 20941: Add unit tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 25 Andrew Isherwood 2018-10-05 14:27:12 UTC
Created attachment 80114 [details] [review]
Bug 20941: (follow-up) Carry out comment #4 advice

- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 26 Andrew Isherwood 2018-10-05 14:27:15 UTC
Created attachment 80115 [details] [review]
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 27 Andrew Isherwood 2018-10-05 14:27:19 UTC
Created attachment 80116 [details] [review]
Bug 20941: (follow-up) Wrap N/A strings in <span>

All N/A strings in ILL templates wrapped in <span> to aid translation
Comment 28 Andrew Isherwood 2018-10-05 14:27:23 UTC
Created attachment 80117 [details] [review]
Bug 20941: (follow-up) Update N/A handling

Modify the conditional display of N/A in accordance with comment #17
Comment 29 Katrin Fischer 2018-10-05 15:13:50 UTC
Created attachment 80127 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 30 Katrin Fischer 2018-10-05 15:13:55 UTC
Created attachment 80128 [details] [review]
Bug 20941: Add unit tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 31 Katrin Fischer 2018-10-05 15:13:59 UTC
Created attachment 80129 [details] [review]
Bug 20941: (follow-up) Carry out comment #4 advice

- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 32 Katrin Fischer 2018-10-05 15:14:03 UTC
Created attachment 80130 [details] [review]
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 33 Katrin Fischer 2018-10-05 15:14:07 UTC
Created attachment 80131 [details] [review]
Bug 20941: (follow-up) Wrap N/A strings in <span>

All N/A strings in ILL templates wrapped in <span> to aid translation
Comment 34 Katrin Fischer 2018-10-05 15:14:12 UTC
Created attachment 80132 [details] [review]
Bug 20941: (follow-up) Update N/A handling

Modify the conditional display of N/A in accordance with comment #17
Comment 35 Katrin Fischer 2018-10-05 15:32:40 UTC
There are some weird things going on:

- Add a new requst with free form
- Edit item metadata
- The Type pull down is empty

This seems to be a spelling mismatch. The type for new requests saves with the first letter being lower case (article) while the the pull downs look for (Article)

I am also not clear on the switch from medium to type, there are still lines like these in the code:     media      => [ "Book", "Article", "Journal" ], (ill-requests.pl)

Can we clear this up?

It's also a little odd that the rquest type shows in 2 places, in the "Request details" and the "Details from supplier" sections.
Comment 36 Andrew Isherwood 2018-10-08 12:42:00 UTC
Created attachment 80212 [details] [review]
Bug 20941: (follow-up) Switch 'media' -> 'types'

Clearing up the inconsistency mentioned in comment #35. There is one
place where we use the term 'media' for a template variable, everywhere
we refer to material types as 'types'.

NOTE: This is a breaking change for existing backends that still use
'media'. Of the PTFS Europe backends, only the Koha backend uses it,
this will be modified as necessary. Generally backends will supply this
variable themselves, so the breaking-ness of this change should be
minimal.

No test plan as it's backend dependent.
Comment 37 Andrew Isherwood 2018-10-08 14:19:12 UTC
Responses to comment #35:

>This seems to be a spelling mismatch. The type for new requests saves with the first letter being lower case (article) while the the pull downs look for (Article)
This was a bug in the FreeForm backend and has now been fixed. Details: https://github.com/PTFS-Europe/koha-ill-freeform/commit/ddff82bc9adc4bd7205192543b6e18c4ad844bb0

>I am also not clear on the switch from medium to type, there are still lines like these in the code:     media      => [ "Book", "Article", "Journal" ], (ill-requests.pl)
This has been fixed, see comment #36

>It's also a little odd that the rquest type shows in 2 places, in the "Request details" and the "Details from supplier" sections.
The details listed here are all metadata relating to the request and will be dependent on the backend from which the request was made, some backends (BLDSS for example) won't even include "type". Whether this display is even necessary is debatable, since bug 20561 added a modal that displays all this information, it just formats it a bit less nicely. I think there's room for improvement in this area, but would suggest it as a future enhancement rather than addressing it here.
Comment 38 Katrin Fischer 2018-10-29 06:36:07 UTC
Hi Andrew, switching back to signed-off with your follow-up
Comment 39 Josef Moravec 2018-10-31 09:53:22 UTC
Created attachment 81688 [details] [review]
Bug 20941: Add Illrequest->getType method

This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object

To observe broken state:

- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column is not populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row is not populated

To Test:

- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
  - => TEST: Observe that the "Request type" column IS populated
  - Click on "View" for one of the requests
  - => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
  - => TEST: Observer that the "Request type" row IS populated

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Josef Moravec 2018-10-31 09:53:29 UTC
Created attachment 81689 [details] [review]
Bug 20941: Add unit tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2018-10-31 09:53:36 UTC
Created attachment 81690 [details] [review]
Bug 20941: (follow-up) Carry out comment #4 advice

- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2018-10-31 09:53:44 UTC
Created attachment 81691 [details] [review]
Bug 20941: (QA follow-up) Fix return value of get_type, make templates more consistant

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Josef Moravec 2018-10-31 09:53:53 UTC
Created attachment 81692 [details] [review]
Bug 20941: (follow-up) Wrap N/A strings in <span>

All N/A strings in ILL templates wrapped in <span> to aid translation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 44 Josef Moravec 2018-10-31 09:54:00 UTC
Created attachment 81693 [details] [review]
Bug 20941: (follow-up) Update N/A handling

Modify the conditional display of N/A in accordance with comment #17

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 45 Josef Moravec 2018-10-31 09:54:05 UTC
Created attachment 81694 [details] [review]
Bug 20941: (follow-up) Switch 'media' -> 'types'

Clearing up the inconsistency mentioned in comment #35. There is one
place where we use the term 'media' for a template variable, everywhere
we refer to material types as 'types'.

NOTE: This is a breaking change for existing backends that still use
'media'. Of the PTFS Europe backends, only the Koha backend uses it,
this will be modified as necessary. Generally backends will supply this
variable themselves, so the breaking-ness of this change should be
minimal.

No test plan as it's backend dependent.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 46 Nick Clemens 2018-10-31 15:04:05 UTC
Awesome work all!

Pushed to master for 18.11