Bug 23230 - Make Koha::Plugins::Base::_version_compare OO
Summary: Make Koha::Plugins::Base::_version_compare OO
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 20669
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-28 15:08 UTC by Tomás Cohen Arazi
Modified: 2020-11-30 21:45 UTC (History)
4 users (show)

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


Attachments
Bug 23230: Unit tests (2.50 KB, patch)
2019-07-01 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23230: Make _version_compare OO context aware (1.44 KB, patch)
2019-07-01 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23230: (follow-up) Fix broken tests (977 bytes, patch)
2019-07-03 02:36 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23230: Unit tests (2.56 KB, patch)
2019-07-03 02:36 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23230: Make _version_compare OO context aware (1.49 KB, patch)
2019-07-03 02:36 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23230: (follow-up) Fix broken tests (1.01 KB, patch)
2019-07-03 02:36 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23230: Unit tests (2.63 KB, patch)
2019-07-03 11:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23230: Make _version_compare OO context aware (1.56 KB, patch)
2019-07-03 11:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23230: (follow-up) Fix broken tests (1.08 KB, patch)
2019-07-03 11:17 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2019-06-28 15:08:56 UTC
This method is to be run inside plugin objects that inherit from Koha::Plugins::Base, but calling

  $self->_version_compare();

won't work because the method doesn't expect $self to be passed.
Comment 1 Kyle M Hall 2019-07-01 13:13:29 UTC
(In reply to Tomás Cohen Arazi from comment #0)
> This method is to be run inside plugin objects that inherit from
> Koha::Plugins::Base, but calling
> 
>   $self->_version_compare();
> 
> won't work because the method doesn't expect $self to be passed.

We should make it work either way since historically it has been non-OO and there are some plugins that may be using it in a non-OO way. I think ti should be easy to check the first parameter to know if it is being called as an object method or not!
Comment 2 Tomás Cohen Arazi 2019-07-01 14:45:55 UTC
Created attachment 91152 [details] [review]
Bug 23230: Unit tests
Comment 3 Tomás Cohen Arazi 2019-07-01 14:45:58 UTC
Created attachment 91153 [details] [review]
Bug 23230: Make _version_compare OO context aware

On using _version_compare in plugins development, it would be nice if it
was able to be called as an object method. This way we don't need to
fully qualify it like this:

    Koha::Plugins::Base::_version_compare

and we can do

    $self->_version_compare

instead.

This patch implements this behavior change. It is backwards compatible
so plugins using the 'old way' don't break.

To test:
- Apply the unit tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Plugins.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Plugins.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 4 Mark Tompsett 2019-07-03 02:36:02 UTC
Created attachment 91201 [details] [review]
Bug 23230: (follow-up) Fix broken tests

Before patch, the new tests don't run.
After patch, they do.
Comment 5 Mark Tompsett 2019-07-03 02:36:48 UTC
Created attachment 91202 [details] [review]
Bug 23230: Unit tests

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 Mark Tompsett 2019-07-03 02:36:50 UTC
Created attachment 91203 [details] [review]
Bug 23230: Make _version_compare OO context aware

On using _version_compare in plugins development, it would be nice if it
was able to be called as an object method. This way we don't need to
fully qualify it like this:

    Koha::Plugins::Base::_version_compare

and we can do

    $self->_version_compare

instead.

This patch implements this behavior change. It is backwards compatible
so plugins using the 'old way' don't break.

To test:
- Apply the unit tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Plugins.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Plugins.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 7 Mark Tompsett 2019-07-03 02:36:53 UTC
Created attachment 91204 [details] [review]
Bug 23230: (follow-up) Fix broken tests

Before patch, the new tests don't run.
After patch, they do.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 8 Tomás Cohen Arazi 2019-07-03 02:55:24 UTC
(In reply to M. Tompsett from comment #4)
> Created attachment 91201 [details] [review] [review]
> Bug 23230: (follow-up) Fix broken tests
> 
> Before patch, the new tests don't run.
> After patch, they do.

I wonder what bug I attached my version of this patch he he
Comment 9 Kyle M Hall 2019-07-03 11:17:16 UTC
Created attachment 91220 [details] [review]
Bug 23230: Unit tests

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2019-07-03 11:17:25 UTC
Created attachment 91221 [details] [review]
Bug 23230: Make _version_compare OO context aware

On using _version_compare in plugins development, it would be nice if it
was able to be called as an object method. This way we don't need to
fully qualify it like this:

    Koha::Plugins::Base::_version_compare

and we can do

    $self->_version_compare

instead.

This patch implements this behavior change. It is backwards compatible
so plugins using the 'old way' don't break.

To test:
- Apply the unit tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Plugins.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Plugins.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2019-07-03 11:17:27 UTC
Created attachment 91222 [details] [review]
Bug 23230: (follow-up) Fix broken tests

Before patch, the new tests don't run.
After patch, they do.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Martin Renvoize 2019-07-03 13:08:45 UTC
Nice work!

Pushed to master for 19.11.00
Comment 13 Martin Renvoize 2019-07-09 10:31:00 UTC
Nice work!

Pushed to master for 19.11.00
Comment 14 Fridolin Somers 2019-07-25 13:17:36 UTC
This is very useful for plugins so I choose to backport to stable.

Pushed to 19.05.x for 19.05.03

Note this commit in master is not needed for 19.05.x :
  Bug 23230: (RM follow-up) Add rollback to plugin test
Comment 15 Lucas Gass 2019-07-26 15:26:39 UTC
everything applied cleanly and passed test so i am backporting this enchancment 


backported to 18.11.x for 18.11.09