Summary: | Koha::Object->messages must be renamed | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andrew, 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
|
|
Circulation function: | |||
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
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 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> Rebased and signed. Trivial change, 'messages()' was cleary too generic. Good catch, Jonathan! 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 (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? (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 :) 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> Pushed to master for 22.05, thanks to everybody involved [U+1F984] Should we also rename Koha::Object::add_message ? Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.11 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |