*** Bug 32897 has been marked as a duplicate of this bug. ***

If the follow-up commit needs adding to its own bug then let me know and I'll happily do so, I just couldn't get the QA script to pass for that file without tidying the whole file

At a glance, it looks like this data could leak out to the public through the API. How might that be prevented?

(In reply to David Cook from comment #8)
> At a glance, it looks like this data could leak out to the public through
> the API. How might that be prevented?

Then again... while it's available via REST API as exposed by the OPAC website... it looks like you need to be a logged in staff user to see it. 

If an API consumer is showing items, hopefully they're using a public endpoint like http://kohadev.mydnsname.org:8080/api/v1/public/biblios/29/items 

So maybe it's fine...

http://kohadev.mydnsname.org:8080/api/v1/items/73

{"error":"Authorization failure. Missing required permission(s).","required_permissions":{"catalogue":"1"}}

I think the staff permissions should be sufficient protection, this is currently the case on the items table

What was the conclusion here in the end?  Do we need a rebase to resume testing or were we considering using additional fields as an alternative..  and David, were you happy with the permissions response?

(In reply to Martin Renvoize from comment #11)
> and David, were you happy with the permissions response?

More or less heh.

I think we have some significant design flaws with the REST API (particularly in terms of admin API vs public API), but that's out of scope for this one I think.

Rebased and managed to fix the QA tests without tidying the whole Items.pm file

Hi,
I realized the test plan, but at the step 15) I have a message that say "No items modified.". The good item is found, but is not modified.

I've rebased and added missing CRSF protection. Test plan works fine for me now, would you be able to try again? Thanks!

Hi,
It works perfectly, thank you!

I'm getting some odd behavior:

t/db_dependent/api/v1/items/bundled_items.t (Wstat: 512 (exited 2) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 2
  Parse errors: Bad plan.  You planned 2 tests but ran 1.

***************************************************************************
DBIx::Class::Relationship::ManyToMany::many_to_many():
***************************************************************************
The many-to-many relationship 'bundle_items' is trying to create a utility method
called set_bundle_items.
This will completely overwrite one such already existing method on class
Koha::Schema::Result::Item.

You almost certainly want to rename your method or the many-to-many
relationship, as the functionality of the original method will not be
accessible anymore.

To disable this warning set to a true value the environment variable
DBIC_OVERWRITE_HELPER_METHODS_OK

(In reply to Kyle M Hall from comment #23)
> I'm getting some odd behavior:
> 
> t/db_dependent/api/v1/items/bundled_items.t (Wstat: 512 (exited 2) Tests: 1
> Failed: 1)
>   Failed test:  1
>   Non-zero exit status: 2
>   Parse errors: Bad plan.  You planned 2 tests but ran 1.
> 
> ***************************************************************************
> DBIx::Class::Relationship::ManyToMany::many_to_many():
> ***************************************************************************
> The many-to-many relationship 'bundle_items' is trying to create a utility
> method
> called set_bundle_items.
> This will completely overwrite one such already existing method on class
> Koha::Schema::Result::Item.
> 
> You almost certainly want to rename your method or the many-to-many
> relationship, as the functionality of the original method will not be
> accessible anymore.
> 
> To disable this warning set to a true value the environment variable
> DBIC_OVERWRITE_HELPER_METHODS_OK

I can't recreate this, nor can I find a method anywhere called set_bundle_items
I installed the patch and ran a reset_all and the test passes fine - is there some additional config you have that I need to check with?

I've rebased one of the commits to fix some tidying issues on the QA tests and add Baptiste's sign off line to all the commits rather than just the final one

> I can't recreate this, nor can I find a method anywhere called
> set_bundle_items
> I installed the patch and ran a reset_all and the test passes fine - is
> there some additional config you have that I need to check with?

It could be something about my setup. Let's go back to SO and see if another QA'er can have a go at it!

Created attachment 169503 [details] [review]
Bug 19316: Add sort1 and sort2 to items and deleteditems tables

This patch adds to new fields to the items table to allow input for additional data into item records to help with sorting

Sponsored-by: PTFS Europe
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 169504 [details] [review]
Bug 19316: Add DBIC files

Signed-off-by: baptiste <baptiste.bayche@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 169505 [details] [review]
Bug 19316: Add sort1 and sort2 fields to interface and allow editing

This patch adds the new sort1 and sort2 fields to the interface so that they can be viewed and edited

Test plan:
1) Apply patch and reset_all
2) In the catalog, search for a record with at least one item
3) In the Holdings table, click on the barcode for an item. You should now be on /cgi-bin/koha/catalogue/moredetail.pl
4) Scroll down and observe that there is now a section called "Sort fields" with two fields, Sort 1 and Sort 2.
5) Enter some text in the textarea for Sort 1 (max 50 characters) and click Update
6) The page will refresh and your text should still be visible. Check that item in the database and the text should be stored in the sort1 column.
7) Repeat steps 5-6 for Sort 2.
8) Copy the barcode and navigate to Cataloging > Batch item modification
9) Enter the barcode in the textarea under "Or scan items one by one" (You can add multiple bar codes here if you wish)
10) Fill in the Required values (Home library, Current library, Koha item type)
11) In "Other attributes" there should be two fields, Sort 1 and Sort 2
12) Enter some text into both these fields
13) Click save and a job will be enqueued
14) Click "view detail of the enqueued job"
15) A message should be shown on completion that says "X item(s) modified (with Y field(s) modified)." X should match the number of barcodes that you entered into the batch mod and Y should equal X x 2
16) Check the item either in the interface or the database and the text you entered should now be stored and visible
17) Repeat steps 8 - 14 but this time only fill in Sort 1
18) This time X should equal the number of barcodes that you entered into the batch mod but Y should equal X as only one field was filled in.
19) Again, the text you entered should now be stored on the item record

Signed-off-by: baptiste <baptiste.bayche@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 169506 [details] [review]
Bug 19316: Update items api definition

Signed-off-by: baptiste <baptiste.bayche@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 169507 [details] [review]
Bug 19316: Add CSRF protection

Signed-off-by: baptiste <baptiste.bayche@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Hi Nick,

some questions:

1) It looks like these fields for items are MARC independent. They appear in the forms independent of a mapping to 952. Is this correct?

2) How would you import or export this data?

3) What would happen if they were mapped to a MARC subfield? (not in test plan)