Bug 23230

Summary: Make Koha::Plugins::Base::_version_compare OO
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, kyle, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on: 20669    
Bug Blocks:    
Attachments: Bug 23230: Unit tests
Bug 23230: Make _version_compare OO context aware
Bug 23230: (follow-up) Fix broken tests
Bug 23230: Unit tests
Bug 23230: Make _version_compare OO context aware
Bug 23230: (follow-up) Fix broken tests
Bug 23230: Unit tests
Bug 23230: Make _version_compare OO context aware
Bug 23230: (follow-up) Fix broken tests

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