Bug 23463 - Move C4::Items CRUD subroutines to Koha::Item
Summary: Move C4::Items CRUD subroutines to Koha::Item
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords: rel_20_05_target
Depends on:
Blocks: 8132 18252 24107
  Show dependency treegraph
 
Reported: 2019-08-14 15:32 UTC by Jonathan Druart
Modified: 2019-12-13 09:38 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23463: Replace SearchItemsByField by SearchItems (2.25 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove uneeded SearchItemsByField subroutine (1.30 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: fix for testbuilder - set default value items.damaged=0 (756 bytes, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Replace AddItem calls with Koha::Item->store (79.13 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove _set_defaults_for_add (2.32 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Replace _set_derived_columns_for_add (1.98 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: AddItem removing (5.09 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Replace ModItem with Koha::Item->store (51.34 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: remove trivial UpdateHoldingbranch sub (1.66 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Move plugin hook (3.03 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: _set_derived_columns_for_mod was only used for cn_sort (4.02 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: _do_column_fixes_for_mod dealt with integer location and timestamp moved (2.02 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove _mod_item_dates (1.54 KB, patch)
2019-12-02 12:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove ModItem (2.91 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove DelItem (7.46 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove DelItemCheck and ItemSafeToDelete (15.65 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Remove no longer needed subs related to default values (19.41 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Fix CancelReceipt.t (4.09 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Fix failing tests (1.49 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Getting less item-level_itypes warnings (?) (1.81 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Fix AutomaticItemModificationByAge.t (790 bytes, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: update permanent_location if location has been updated (2.94 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Fix items.cn_source vs cn_source (1.93 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: build cn_sort only if needed (1.25 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: Replace recently added occurrences (3.64 KB, patch)
2019-12-02 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: cn_sort needs to be updated if cn_source changed (928 bytes, patch)
2019-12-02 13:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23463: (follow-up) Remove DelItemCheck and ItemSafeToDelete (1005 bytes, patch)
2019-12-10 11:58 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 Jonathan Druart 2019-08-14 15:32:13 UTC

    
Comment 1 Jonathan Druart 2019-08-14 16:06:30 UTC
This patchset will be quite big and will move the following C4::Items subroutines to the Koha namespace:

AddItem
ModItem
DelItem
ItemSafeToDelete
DelItemCheck

SearchItemsByField
UpdateHoldingbranch

_koha_new_item
_koha_modify_item
_set_defaults_for_add
_set_derived_columns_for_add
_do_column_fixes_for_mod
_set_derived_columns_for_mod
_build_default_values_for_mod_marc
_get_single_item_column
_calc_items_cn_sort
_mod_item_dates
_after_item_action_hooks
Comment 2 Jonathan Druart 2019-08-14 16:07:36 UTC
I have pushed a WIP branch to: https://gitlab.com/joubu/Koha/commits/bug_23463

The branch is ready for testing (but still not clean enough to be integrated).

Early feedbacks welcomed!
Comment 3 Jonathan Druart 2019-10-24 15:57:07 UTC
Remote branch has been rebased.
Comment 4 Martin Renvoize 2019-11-01 12:04:44 UTC
Lets hit this one hard beginning of next cycle :)
Comment 5 Jonathan Druart 2019-12-02 12:04:13 UTC
Created attachment 95899 [details] [review]
Bug 23463: Replace SearchItemsByField by SearchItems

The subroutine SearchItemsByField is only called once and can be
replaced easily with SearchItems
Comment 6 Jonathan Druart 2019-12-02 12:04:16 UTC
Created attachment 95900 [details] [review]
Bug 23463: Remove uneeded SearchItemsByField subroutine

It is no longer used.
Comment 7 Jonathan Druart 2019-12-02 12:04:19 UTC
Created attachment 95901 [details] [review]
Bug 23463: fix for testbuilder - set default value items.damaged=0
Comment 8 Jonathan Druart 2019-12-02 12:04:24 UTC
Created attachment 95902 [details] [review]
Bug 23463: Replace AddItem calls with Koha::Item->store
Comment 9 Jonathan Druart 2019-12-02 12:04:28 UTC
Created attachment 95903 [details] [review]
Bug 23463: Remove _set_defaults_for_add

_set_defaults_for_add is no longer needed as it is handled by dbic and koha::object->store
Comment 10 Jonathan Druart 2019-12-02 12:04:31 UTC
Created attachment 95904 [details] [review]
Bug 23463: Replace _set_derived_columns_for_add

Remove the unique call to _set_derived_columns_for_add
Even if design to deal with other values it only dealts with cn_sort
Comment 11 Jonathan Druart 2019-12-02 12:04:34 UTC
Created attachment 95905 [details] [review]
Bug 23463: AddItem removing

We are done with AddItem, only need to log and index.
Comment 12 Jonathan Druart 2019-12-02 12:04:38 UTC
Created attachment 95906 [details] [review]
Bug 23463: Replace ModItem with Koha::Item->store

Starting to replace the ModItem calls with Koha::Item->store
Comment 13 Jonathan Druart 2019-12-02 12:04:42 UTC
Created attachment 95907 [details] [review]
Bug 23463: remove trivial UpdateHoldingbranch sub

We do not need this subroutine, moving it where it is used.
Comment 14 Jonathan Druart 2019-12-02 12:04:46 UTC
Created attachment 95908 [details] [review]
Bug 23463: Move plugin hook

From C4::Items to Koha::Item
Comment 15 Jonathan Druart 2019-12-02 12:04:50 UTC
Created attachment 95909 [details] [review]
Bug 23463: _set_derived_columns_for_mod was only used for cn_sort

Same as _set_defaults_for_add
Comment 16 Jonathan Druart 2019-12-02 12:04:53 UTC
Created attachment 95910 [details] [review]
Bug 23463: _do_column_fixes_for_mod dealt with integer location and timestamp moved

That's done in Koha::Item->store, and Koha::Object->store for integers.
Comment 17 Jonathan Druart 2019-12-02 12:04:56 UTC
Created attachment 95911 [details] [review]
Bug 23463: Remove _mod_item_dates

We did not do anything useful here.
Comment 18 Jonathan Druart 2019-12-02 12:05:00 UTC
Created attachment 95912 [details] [review]
Bug 23463: Remove ModItem

There should not be any calls left
Comment 19 Jonathan Druart 2019-12-02 12:05:04 UTC
Created attachment 95913 [details] [review]
Bug 23463: Remove DelItem
Comment 20 Jonathan Druart 2019-12-02 12:05:08 UTC
Created attachment 95914 [details] [review]
Bug 23463: Remove DelItemCheck and ItemSafeToDelete
Comment 21 Jonathan Druart 2019-12-02 12:05:12 UTC
Created attachment 95915 [details] [review]
Bug 23463: Remove no longer needed subs related to default values
Comment 22 Jonathan Druart 2019-12-02 12:05:15 UTC
Created attachment 95916 [details] [review]
Bug 23463: Fix CancelReceipt.t
Comment 23 Jonathan Druart 2019-12-02 12:05:18 UTC
Created attachment 95917 [details] [review]
Bug 23463: Fix failing tests

And adding a FIXME about discard_changes
Comment 24 Jonathan Druart 2019-12-02 12:05:22 UTC
Created attachment 95918 [details] [review]
Bug 23463: Getting less item-level_itypes warnings (?)

So, we get less warnings. I do not really understand why...
Comment 25 Jonathan Druart 2019-12-02 12:05:27 UTC
Created attachment 95919 [details] [review]
Bug 23463: Fix AutomaticItemModificationByAge.t

We do not want to log if nothing changed

How could we do that better?
Comment 26 Jonathan Druart 2019-12-02 12:05:30 UTC
Created attachment 95920 [details] [review]
Bug 23463: update permanent_location if location has been updated

This change to Koha::Item->store seems correct here, but tests from /db_dependent/Items.t is failing now.

Adjusting them to make them pass.
Comment 27 Jonathan Druart 2019-12-02 12:05:34 UTC
Created attachment 95921 [details] [review]
Bug 23463: Fix items.cn_source vs cn_source
Comment 28 Jonathan Druart 2019-12-02 12:05:37 UTC
Created attachment 95922 [details] [review]
Bug 23463: build cn_sort only if needed

No need to calculate cn_sort if not dirty when we store
Comment 29 Jonathan Druart 2019-12-02 12:05:41 UTC
Created attachment 95923 [details] [review]
Bug 23463: Replace recently added occurrences

Few occurrences have been added since this patchset has been originaly
written
Comment 30 Katrin Fischer 2019-12-02 12:17:41 UTC
Comment on attachment 95922 [details] [review]
Bug 23463: build cn_sort only if needed

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

::: Koha/Item.pm
@@ +74,5 @@
>          $self->itype($self->biblio->biblioitem->itemtype);
>      }
>  
> +    my %updated_columns = $self->_result->get_dirty_columns;
> +    if ( exists $updated_columns{itemcallnumber} ) {

We should also update the cn_sort if cn_source was changed.
Comment 31 Jonathan Druart 2019-12-02 13:32:33 UTC
Created attachment 95925 [details] [review]
Bug 23463: cn_sort needs to be updated if cn_source changed
Comment 32 Jonathan Druart 2019-12-02 13:32:48 UTC
(In reply to Katrin Fischer from comment #30)
> Comment on attachment 95922 [details] [review] [review]
> Bug 23463: build cn_sort only if needed
> 
> Review of attachment 95922 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Item.pm
> @@ +74,5 @@
> >          $self->itype($self->biblio->biblioitem->itemtype);
> >      }
> >  
> > +    my %updated_columns = $self->_result->get_dirty_columns;
> > +    if ( exists $updated_columns{itemcallnumber} ) {
> 
> We should also update the cn_sort if cn_source was changed.

Indeed, thanks!
Comment 33 Jonathan Druart 2019-12-10 11:58:39 UTC
Created attachment 96145 [details] [review]
Bug 23463: (follow-up) Remove DelItemCheck and ItemSafeToDelete

Wrong replacement in additem.pl
Comment 34 Martin Renvoize 2019-12-13 09:38:53 UTC
This is on my list for early January