Bug 29785

Summary: Koha::Object->messages must be renamed
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: andrewfh, fridolin.somers, kyle, m.de.rooy, martin.renvoize, tomascohen, victor
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.03,21.05.11
Bug Depends on:    
Bug Blocks: 29230    
Attachments: Bug 29785: Rename Koha::Object->message with ->object_messages
Bug 29785: Rename Koha::Object->message with ->object_messages
Bug 29785: Rename Koha::Object->message with ->object_messages

Description Jonathan Druart 2022-01-04 13:27:58 UTC
It will conflict with other ->messages methods, it's too generic.

Bug 29230 needs Koha::Patron->messages to return Koha::Patron::Messages for instance.
Comment 1 Jonathan Druart 2022-01-04 13:38:13 UTC
Created attachment 128986 [details] [review]
Bug 29785: Rename Koha::Object->message with ->object_messages

It will conflict with other ->messages methods, it's too generic.

Bug 29230 needs Koha::Patron->messages to return Koha::Patron::Messages for instance.

Test plan:
Confirm that the tests modified by this patch still pass
Comment 2 Tomás Cohen Arazi 2022-01-31 16:22:27 UTC
Created attachment 130018 [details] [review]
Bug 29785: Rename Koha::Object->message with ->object_messages

It will conflict with other ->messages methods, it's too generic.

Bug 29230 needs Koha::Patron->messages to return Koha::Patron::Messages for instance.

Test plan:
Confirm that the tests modified by this patch still pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2022-01-31 16:23:03 UTC
Rebased and signed. Trivial change, 'messages()' was cleary too generic. Good catch, Jonathan!
Comment 4 Marcel de Rooy 2022-02-01 12:24:35 UTC
This looks like it still needs attention though:

=head3 messages

    my @messages = @{ $bool->messages };

Returns the I<Koha::Object::Message> objects that were recorded.

=cut

sub messages {
    my ( $self ) = @_;

    $self->{_messages} = []
        unless defined $self->{_messages};

    return $self->{_messages};
}

Kind of strange btw that we add a lot of code there?
Koha::Result::Boolean->new(0)->add_message({ message => $error });

Please provide feedback! Thanks
Comment 5 Jonathan Druart 2022-02-01 13:51:25 UTC
(In reply to Marcel de Rooy from comment #4)
> This looks like it still needs attention though:
> 
> =head3 messages
> 
>     my @messages = @{ $bool->messages };
> 
> Returns the I<Koha::Object::Message> objects that were recorded.
> 
> =cut
> 
> sub messages {
>     my ( $self ) = @_;
> 
>     $self->{_messages} = []
>         unless defined $self->{_messages};
> 
>     return $self->{_messages};
> }

This looks correct to me, Koha::Result::Boolean is not inheriting from Koha::Object.

> Kind of strange btw that we add a lot of code there?
> Koha::Result::Boolean->new(0)->add_message({ message => $error });

Can you detail what you mean here?
Comment 6 Marcel de Rooy 2022-02-01 14:02:32 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to Marcel de Rooy from comment #4)
> This looks correct to me, Koha::Result::Boolean is not inheriting from
> Koha::Object.

Okay

> > Kind of strange btw that we add a lot of code there?
> > Koha::Result::Boolean->new(0)->add_message({ message => $error });
> 
> Can you detail what you mean here?

Forget that. Result here confused me a bit. Surely it not a Schema::Result, but what is a Koha::Result actually?
No answer needed here :)
Comment 7 Marcel de Rooy 2022-02-01 14:05:47 UTC
Created attachment 130043 [details] [review]
Bug 29785: Rename Koha::Object->message with ->object_messages

It will conflict with other ->messages methods, it's too generic.

Bug 29230 needs Koha::Patron->messages to return Koha::Patron::Messages for instance.

Test plan:
Confirm that the tests modified by this patch still pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Fridolin Somers 2022-02-10 08:03:25 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 9 Fridolin Somers 2022-02-10 08:06:27 UTC
Should we also rename Koha::Object::add_message ?
Comment 10 Kyle M Hall 2022-02-11 11:40:10 UTC
Pushed to 21.11.x for 21.11.03
Comment 11 Andrew Fuerste-Henry 2022-02-21 16:38:27 UTC
Pushed to 21.05.x for 21.05.11
Comment 12 Victor Grousset/tuxayo 2022-02-24 23:38:08 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.