Bug 32418 - CRASH: Can't call method "unblessed" on an undefined value at cataloguing/additem.pl
Summary: CRASH: Can't call method "unblessed" on an undefined value at cataloguing/add...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Paul Derscheid
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-07 14:37 UTC by Marcel de Rooy
Modified: 2023-12-28 20:43 UTC (History)
5 users (show)

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


Attachments
Add fallback redirect to additem op if \$current_item is undefined (1.67 KB, patch)
2022-12-14 10:36 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418 - Can't call method "unblessed" on an undefined value at cataloguing/additem.pl (1.67 KB, patch)
2022-12-16 08:50 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (1.88 KB, patch)
2022-12-16 08:59 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32420: Set focus for cursor to barcode input box in batch item modification (1.46 KB, text/plain)
2022-12-16 10:25 UTC, Paul Derscheid
Details
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (1.96 KB, patch)
2022-12-16 11:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (5.24 KB, patch)
2022-12-22 13:52 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (5.25 KB, patch)
2022-12-23 09:11 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (5.29 KB, patch)
2023-01-13 19:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (5.29 KB, patch)
2023-01-13 19:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (5.92 KB, patch)
2023-01-20 10:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (4.74 KB, patch)
2023-02-20 10:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (4.77 KB, patch)
2023-04-03 10:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param (2.96 KB, patch)
2023-04-13 09:30 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (4.77 KB, patch)
2023-04-13 21:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param (2.99 KB, patch)
2023-04-13 21:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl (4.84 KB, patch)
2023-04-14 06:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param (3.06 KB, patch)
2023-04-14 06:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32418: Remove useless statement (891 bytes, patch)
2023-04-14 06:24 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 Marcel de Rooy 2022-12-07 14:37:08 UTC
21.11 line 356
$op eq "edititem"
*     $current_item = Koha::Items->find($itemnumber)->unblessed;

Master L465
Comment 1 Paul Derscheid 2022-12-14 10:36:56 UTC
Created attachment 144571 [details] [review]
Add fallback redirect to additem op if \$current_item is undefined
Comment 2 Marcel de Rooy 2022-12-16 07:37:02 UTC
Just nagging:

Assignee not filled.
QA tools warn: * Commit title does not start with 'Bug XXXXX: ' - 672a0dd335
Comment 3 Marcel de Rooy 2022-12-16 07:38:54 UTC
+    $current_item = Koha::Items->find($itemnumber)->unblessed or undef;

This does not work. The find will not return an opbject. So the unblessed still crashes.
Comment 4 Marcel de Rooy 2022-12-16 07:40:27 UTC
+    if (!$current_item) { $nextop = "additem"; last; }

What does this last do here? You dont exit a loop here..
Comment 5 Paul Derscheid 2022-12-16 08:23:31 UTC
Yeah, sorry. Fixin' it now.
Comment 6 Paul Derscheid 2022-12-16 08:50:07 UTC
Created attachment 144639 [details] [review]
Bug 32418 - Can't call method "unblessed" on an undefined value at cataloguing/additem.pl

Check whether result of Koha::Items->find is defined before invoking unblessed. If undef, redirect to additem op.
Comment 7 Paul Derscheid 2022-12-16 08:59:15 UTC
Created attachment 144640 [details] [review]
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether result of Koha::Items->find is defined before invoking unblessed. If undef, redirect to additem op.
Comment 8 Paul Derscheid 2022-12-16 09:08:19 UTC
This time I actually tested it. Should work now. But still open for ideas.
Comment 9 Paul Derscheid 2022-12-16 10:25:55 UTC
Created attachment 144641 [details]
Bug 32420: Set focus for cursor to barcode input box in batch item modification

This should do it. Don't know whether it's compatible with the JS guidelines in the wiki.
But I don't know whether it makes that much sense to move this block to
one of the included scripts.

To test:
1. Go to Cataloging > Batch item modification
	--> The barcodelist input field is now selected by default in
the 'batch item modification' as well as the 'batch item deletion' view.
Comment 10 Marcel de Rooy 2022-12-16 10:28:10 UTC
(In reply to Paul Derscheid from comment #9)
> Created attachment 144641 [details] [review]
> Bug 32420: Set focus for cursor to barcode input box in batch item
> modification
> 
> This should do it. Don't know whether it's compatible with the JS guidelines
> in the wiki.
> But I don't know whether it makes that much sense to move this block to
> one of the included scripts.
> 
> To test:
> 1. Go to Cataloging > Batch item modification
> 	--> The barcodelist input field is now selected by default in
> the 'batch item modification' as well as the 'batch item deletion' view.

Wrong bug !
Comment 11 Marcel de Rooy 2022-12-16 10:30:04 UTC
+    if (!$current_item) { $nextop = "additem"; return; }

Return ?
Comment 12 Paul Derscheid 2022-12-16 11:20:47 UTC
Created attachment 144643 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether result of Koha::Items->find is defined before invoking unblessed. If undef, redirect to additem op.

This uses an unlabeled block so we can add a guard clause to the 'dupeitem' case and exit early with the 'last' keyword.
Comment 13 Fridolin Somers 2022-12-22 06:15:27 UTC
This needs a test plan.

I can reproduce by editing URL with a unkown itemnumber, ie :
/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999 (with KTD).

But I think we should test the item existence at the beginning of the page.
And display a message in TT, like in addbiblio.pl :
https://git.koha-community.org/Koha-community/Koha/src/commit/0a934194ca25e2614912f9248f55822aebf38fb8/cataloguing/addbiblio.pl#L771
Comment 14 Paul Derscheid 2022-12-22 13:52:21 UTC
Created attachment 144794 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off
Comment 15 Fridolin Somers 2022-12-22 20:14:56 UTC
Great code Paul ;)

If I may :

I didnt saw there is a method output_and_exit_if_error() that manages incorrect patron + biblio.
Maybe it would be nice to move you check on item in it ?

Is uses include blocking_errors.inc that already contains "unknown_item" code, see :

https://git.koha-community.org/Koha-community/Koha/src/commit/9d6d641d1f8b77271800f43bc027b651f9aea52b/circ/bookcount.pl#L50
Comment 16 Paul Derscheid 2022-12-23 09:11:11 UTC
Created attachment 144812 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If the item is not defined we output_and_exit and save the time to
execute the rest of the controller code. Additionally we now don't have
to process the whole template anymore.

The only caveat is that we now have the suggested actions above the
actual error message. Still an improvement. though.

Again, thanks Frido. Hope, that this is what you had in mind.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off
Comment 17 David Nind 2023-01-13 02:37:47 UTC
I'm getting a sha1 error when trying to apply the patch to test (I was using interactive and only applying the latest patch).

I think the first patch needs obsoleting.

Message when trying to apply the patch:

git bz apply 32418

Bug 32418 - CRASH: Can't call method "unblessed" on an undefined value at cataloguing/additem.pl

144794 - Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl
144812 - Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Apply? [(y)es, (n)o, (i)nteractive] i
Applying: Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl
error: sha1 information is lacking or useless (cataloguing/additem.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl
Comment 18 Paul Derscheid 2023-01-13 12:21:50 UTC
The second patch relies on changes in the first.
Works correctly if you apply both.
Comment 19 David Nind 2023-01-13 18:35:53 UTC
(In reply to Paul Derscheid from comment #18)
> The second patch relies on changes in the first.
> Works correctly if you apply both.

Thanks Paul. I have done that now and will sign off.

I was a bit confused as the patch titles and test plan were the same/almost identical, and I thought (without actually looking at the patches) that they were the same.

You may want to amend the patches for the QA Team so that the subject line is in the format Bug XXXXX: Bug title, as per https://wiki.koha-community.org/wiki/Commit_messages
Comment 20 David Nind 2023-01-13 19:46:38 UTC
Created attachment 145306 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2023-01-13 19:46:44 UTC
Created attachment 145307 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If the item is not defined we output_and_exit and save the time to
execute the rest of the controller code. Additionally we now don't have
to process the whole template anymore.

The only caveat is that we now have the suggested actions above the
actual error message. Still an improvement. though.

Again, thanks Frido. Hope, that this is what you had in mind.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 Marcel de Rooy 2023-01-20 10:03:29 UTC
Two patches with the same commit name. Will squash them.
Comment 23 Marcel de Rooy 2023-01-20 10:04:09 UTC
Created attachment 145505 [details] [review]
Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>

Bug 32418 - Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If the item is not defined we output_and_exit and save the time to
execute the rest of the controller code. Additionally we now don't have
to process the whole template anymore.

The only caveat is that we now have the suggested actions above the
actual error message. Still an improvement. though.

Again, thanks Frido. Hope, that this is what you had in mind.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 Marcel de Rooy 2023-01-20 10:04:45 UTC
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
   FAIL   filters
                wrong_html_filter at line 71 (                        <a href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% biblio.biblionumber %]">Add a new item</a> or <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber %]#holdings">go to the records holdings</a>.)
                missing_filter at line 71 (                        <a href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% biblio.biblionumber %]">Add a new item</a> or <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber %]#holdings">go to the records holdings</a>.)
                missing_filter at line 71 (                        <a href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% biblio.biblionumber %]">Add a new item</a> or <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber %]#holdings">go to the records holdings</a>.)
Comment 25 Marcel de Rooy 2023-01-20 10:05:08 UTC
Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - 94c83e2a74
Comment 26 Fridolin Somers 2023-01-20 20:25:25 UTC
(In reply to Paul Derscheid from comment #18)
> The second patch relies on changes in the first.
> Works correctly if you apply both.

In this cas you need to add "(follow-up)" in second patch.
See for example Bug 32283

More there https://wiki.koha-community.org/wiki/Commit_messages

With 1rst rule being :
Bug XXX - blabla
=>
Bug XXX: blabla
Comment 27 Fridolin Somers 2023-01-20 20:26:49 UTC
(In reply to Marcel de Rooy from comment #24)
>  FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
>    FAIL   filters
>                 wrong_html_filter at line 71 (                        <a
> href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[%
> biblio.biblionumber %]">Add a new item</a> or <a
> href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber
> %]#holdings">go to the records holdings</a>.)
>                 missing_filter at line 71 (                        <a
> href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[%
> biblio.biblionumber %]">Add a new item</a> or <a
> href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber
> %]#holdings">go to the records holdings</a>.)
>                 missing_filter at line 71 (                        <a
> href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[%
> biblio.biblionumber %]">Add a new item</a> or <a
> href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber
> %]#holdings">go to the records holdings</a>.)

You need to add Template Toolkit filter for URI :
/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber | uri %]
Comment 28 Fridolin Somers 2023-01-20 20:34:46 UTC
(In reply to Fridolin Somers from comment #15)
> Great code Paul ;)
> 
> If I may :
> 
> I didnt saw there is a method output_and_exit_if_error() that manages
> incorrect patron + biblio.
> Maybe it would be nice to move you check on item in it ?
> 
> Is uses include blocking_errors.inc that already contains "unknown_item"
> code, see :
> 
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> 9d6d641d1f8b77271800f43bc027b651f9aea52b/circ/bookcount.pl#L50

Forget about that ;)

The actual code is close to the code in addbiblio.tt that's perfect.
Comment 29 Paul Derscheid 2023-02-20 10:08:52 UTC
Created attachment 146944 [details] [review]
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off
Comment 30 Katrin Fischer 2023-02-20 20:54:36 UTC
Is this ready for retesting?
Comment 31 Paul Derscheid 2023-04-03 08:55:39 UTC
Hi Katrin, this is ready for retesting.
Comment 32 David Nind 2023-04-03 10:05:53 UTC
Created attachment 149092 [details] [review]
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 33 Jonathan Druart 2023-04-13 07:43:02 UTC
* We don't need to hide nodes in the template
* The links we display in the template does not work (we do not send the biblio to the template as we exit early)
Comment 34 Paul Derscheid 2023-04-13 09:30:15 UTC
Created attachment 149591 [details] [review]
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off
Comment 35 David Nind 2023-04-13 21:27:52 UTC
Created attachment 149626 [details] [review]
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 36 David Nind 2023-04-13 21:27:55 UTC
Created attachment 149627 [details] [review]
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 37 Jonathan Druart 2023-04-14 06:24:32 UTC
Created attachment 149635 [details] [review]
Bug 32418: Can't call method 'unblessed' on an undefined value at cataloguing/additem.pl

Check whether the current op is edititem or dupeitem and if so check
whether the itemnumber supplied as a query param actually exists.

If it doesn't, redirect to the additem op and hide all UI elements except
for a dialog that gives options to add a new item to the record or to
view the records holdings.

This behaviour was adapted from the addbiblio view, as suggested by
Fridolin.

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 38 Jonathan Druart 2023-04-14 06:24:35 UTC
Created attachment 149636 [details] [review]
Bug 32418: (follow-up) Remove jQuery, Fix typos in url, add biblio as template param

To test:
1. Go to any record
2. Hit the edit button on an item in the holdings table
3. Modify the URL so that the query param for the itemnumber is either
    3.1 empty: /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=
    3.2 an itemnumber that doesn't exist:
      /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=384&itemnumber=9999999999
    3.3 whatever else you come up with..
4. Check the same thing for the dupe option (op=dupeitem)
5. Sign off

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 39 Jonathan Druart 2023-04-14 06:24:38 UTC
Created attachment 149637 [details] [review]
Bug 32418: Remove useless statement

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Tomás Cohen Arazi 2023-04-14 14:19:50 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 41 Matt Blenkinsop 2023-04-18 12:06:09 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 42 Lucas Gass 2023-05-05 22:13:01 UTC
Doesn't apply cleanly to 22.05.x, no backport.