[Bug 19938] New: Refactor C4::Overdues::checkoverdues

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] New: Refactor C4::Overdues::checkoverdues

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

            Bug ID: 19938
           Summary: Refactor C4::Overdues::checkoverdues
 Change sponsored?: ---
           Product: Koha
           Version: master
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5 - low
         Component: Circulation
          Assignee: [hidden email]
          Reporter: [hidden email]
        QA Contact: [hidden email]
                CC: [hidden email], [hidden email]

While retesting a bug, I ran a prove -r -v t, which generated a lot of log
noise. One such entry was:
[Tue Jan  9 03:54:58 2018] starman: Use of uninitialized value $overdues_count
in numeric gt (>) at /home/vagrant/kohaclone/opac/opac-main.pl line 76.

The $overdues_count was set by a call to checkoverdues. Reading the code I
noticed someone actually commented:
# returning the count and the results is silly

So, this is a refactor of the function and all the calls. It was surprisingly
small.

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |Needs Signoff

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #1 from M. Tompsett <[hidden email]> ---
Created attachment 70375
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70375&action=edit
Bug 19938: Refactor checkoverdues

TEST PLAN
---------
0) back up plack error log if you care.

1) Run these on your kohadevbox:
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    However, the log file will have noise.

2) Run these on your kohadevbox:
    git bz apply 19938
    restart_all
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    This time the log file will have no noise.

3) run koha qa test tools

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[hidden email]-commun |[hidden email]
                   |ity.org                     |

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |minor

--- Comment #2 from M. Tompsett <[hidden email]> ---
This may conflict with 19933 which removes C4::Members::patronflags if 19933 is
pushed first. In which case, I believe removing C4/Member's from this patch
should be sufficient. I'll split.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #3 from M. Tompsett <[hidden email]> ---
Created attachment 70382
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70382&action=edit
Bug 19938: Split C4/Members.pm to avoid conflict issues

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70375|0                           |1
        is obsolete|                            |

--- Comment #4 from M. Tompsett <[hidden email]> ---
Created attachment 70383
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70383&action=edit
Bug 19938: Refactor checkoverdues

TEST PLAN
---------
0) back up plack error log if you care.

1) Run these on your kohadevbox:
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    However, the log file will have noise.

2) Run these on your kohadevbox:
    git bz apply 19938
    restart_all
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    This time the log file will have no noise.

3) run koha qa test tools

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Lee Jamison <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Signoff               |Signed Off

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Lee Jamison <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70382|0                           |1
        is obsolete|                            |

--- Comment #5 from Lee Jamison <[hidden email]> ---
Created attachment 70396
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70396&action=edit
Bug 19938: Split C4/Members.pm to avoid conflict issues

Signed-off-by: Lee Jamison <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Lee Jamison <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70383|0                           |1
        is obsolete|                            |

--- Comment #6 from Lee Jamison <[hidden email]> ---
Created attachment 70397
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70397&action=edit
Bug 19938: Refactor checkoverdues

TEST PLAN
---------
0) back up plack error log if you care.

1) Run these on your kohadevbox:
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    However, the log file will have noise.

2) Run these on your kohadevbox:
    git bz apply 19938
    restart_all
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    This time the log file will have no noise.

3) run koha qa test tools

Signed-off-by: Lee Jamison <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Jonathan Druart <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]-c
                   |                            |ommunity.org

--- Comment #7 from Jonathan Druart <[hidden email]> ---
(In reply to M. Tompsett from comment #0)
> While retesting a bug, I ran a prove -r -v t, which generated a lot of log
> noise. One such entry was:
> [Tue Jan  9 03:54:58 2018] starman: Use of uninitialized value
> $overdues_count in numeric gt (>) at
> /home/vagrant/kohaclone/opac/opac-main.pl line 76.

Prove generated a log entry from opac-main.pl? How?

https://jenkins.koha-community.org/job/Koha_Master_D8/lastCompletedBuild/console

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #8 from Jonathan Druart <[hidden email]> ---
"Bug 19938: Split C4/Members.pm to avoid conflict issues"

 => What does it mean?

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #9 from M. Tompsett <[hidden email]> ---
(In reply to Jonathan Druart from comment #8)
> "Bug 19938: Split C4/Members.pm to avoid conflict issues"
>
>  => What does it mean?

because 19933 might trigger a conflict in C4/Members with the removal of
patronflags, so put the possible conflict area elsewhere. I split it from a
single commit, so as to avoid the conflict as needed.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Julian Maurice <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
         QA Contact|[hidden email]-communit |[hidden email]
                   |y.org                       |

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #10 from Julian Maurice <[hidden email]> ---
(In reply to M. Tompsett from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > "Bug 19938: Split C4/Members.pm to avoid conflict issues"
> >
> >  => What does it mean?
>
> because 19933 might trigger a conflict in C4/Members with the removal of
> patronflags, so put the possible conflict area elsewhere. I split it from a
> single commit, so as to avoid the conflict as needed.

I don't see how they can conflict. Bug 19933 only adds a comment to
C4/Members.pm, and not very close to the lines modified by this bug's patches.
I think it's safe to squash the patches.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #11 from Julian Maurice <[hidden email]> ---
(In reply to Jonathan Druart from comment #7)

> (In reply to M. Tompsett from comment #0)
> > While retesting a bug, I ran a prove -r -v t, which generated a lot of log
> > noise. One such entry was:
> > [Tue Jan  9 03:54:58 2018] starman: Use of uninitialized value
> > $overdues_count in numeric gt (>) at
> > /home/vagrant/kohaclone/opac/opac-main.pl line 76.
>
> Prove generated a log entry from opac-main.pl? How?
>
> https://jenkins.koha-community.org/job/Koha_Master_D8/lastCompletedBuild/
> console

t/db_dependent/www/search_utf8.t use Test::WWW::Mechanize to do various HTTP
requests to Koha, that's how.

I can confirm that warnings disappear after applying the patches, but before I
mark this as Passed QA, please rewrite the commit message. It's not really a
refactoring because it changes behaviour.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #12 from Julian Maurice <[hidden email]> ---
(In reply to Julian Maurice from comment #10)
> I don't see how they can conflict. Bug 19933 only adds a comment to
> C4/Members.pm, and not very close to the lines modified by this bug's
> patches.
> I think it's safe to squash the patches.

I didn't realize the code modified by this patch was inside patronflags(), so
feel free to squash the patches or not, but if you decide to keep both patches
separated, please rewrite the first commit message too, it doesn't describe
what the patch does

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Julian Maurice <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #13 from M. Tompsett <[hidden email]> ---
Created attachment 70472
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70472&action=edit
Bug 19938: Refactor checkoverdues call in C4/Members.pm

Due to concerns that another bug may conflict, this was
purposefully split. The other bug removes patronflags().
It is this function that has the C4::Overdues::checkoverdues
call refactored.

TEST PLAN
---------
Apply both patches and follow the test plan in the larger one.

Signed-off-by: Lee Jamison <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70397|0                           |1
        is obsolete|                            |

--- Comment #14 from M. Tompsett <[hidden email]> ---
Created attachment 70473
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70473&action=edit
Bug 19938: Refactor checkoverdues

TEST PLAN
---------
0) back up plack error log if you care.

1) Run these on your kohadevbox:
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    However, the log file will have noise.

2) Run these on your kohadevbox:
    git bz apply 19938
    restart_all
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    This time the log file will have no noise.

3) run koha qa test tools

Signed-off-by: Lee Jamison <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70396|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

M. Tompsett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Signed Off

--- Comment #15 from M. Tompsett <[hidden email]> ---
It seemed from comment #11 and comment #12, that the FailedQA was for the git
commit messages, not the actual code.

Changing the return value and tweaking code based on that is a refactor, just
not in a huge rewrite from scratch sense. Perhaps there is a better word, but I
can't think of it right now.

As I only changed the commit message, I'm moving this back to Signed off.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #16 from Julian Maurice <[hidden email]> ---
(In reply to M. Tompsett from comment #15)
> It seemed from comment #11 and comment #12, that the FailedQA was for the
> git commit messages, not the actual code.
>
> Changing the return value and tweaking code based on that is a refactor,
> just not in a huge rewrite from scratch sense. Perhaps there is a better
> word, but I can't think of it right now.
According to Wikipedia,
> Code refactoring is the process of restructuring existing computer code
> without changing its external behavior.
IMO the return value is part of the external behavior of a subroutine, so it's
not a refactor.

But I won't block this any longer for such a minor issue.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Julian Maurice <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70472|0                           |1
        is obsolete|                            |

--- Comment #17 from Julian Maurice <[hidden email]> ---
Created attachment 70485
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70485&action=edit
Bug 19938: Refactor checkoverdues call in C4/Members.pm

Due to concerns that another bug may conflict, this was
purposefully split. The other bug removes patronflags().
It is this function that has the C4::Overdues::checkoverdues
call refactored.

TEST PLAN
---------
Apply both patches and follow the test plan in the larger one.

Signed-off-by: Lee Jamison <[hidden email]>
Signed-off-by: Julian Maurice <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Julian Maurice <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70473|0                           |1
        is obsolete|                            |

--- Comment #18 from Julian Maurice <[hidden email]> ---
Created attachment 70486
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70486&action=edit
Bug 19938: Refactor checkoverdues

TEST PLAN
---------
0) back up plack error log if you care.

1) Run these on your kohadevbox:
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    However, the log file will have noise.

2) Run these on your kohadevbox:
    git bz apply 19938
    restart_all
    echo | sudo tee /var/log/koha/kohadev/plack-error.log
    sudo koha-shell -c bash kohadev
    prove t/db_dependent/Circulation/dateexpiry.t
    prove t/db_dependent/www/search_utf8.t
    ./misc/cronjobs/staticfines.pl
    exit

    They will all pass or run without error.
    This time the log file will have no noise.

3) run koha qa test tools

Signed-off-by: Lee Jamison <[hidden email]>
Signed-off-by: Julian Maurice <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Julian Maurice <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Passed QA

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #19 from Jonathan Druart <[hidden email]> ---
Created attachment 70489
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70489&action=edit
Bug 19938: Do not fetch dashboard's info if no logged in

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

Jonathan Druart <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |In Discussion

--- Comment #20 from Jonathan Druart <[hidden email]> ---
So actually we are trying to remove a warning because there are wrong calls in
opac-main.
The dashboard's info should be fetched only if a patron is logged in.
I did this change on bug 12001 already.
I am suggesting a new patch to avoid the change of the subroutine's prototype
(which is wrong instead, but let remove the subroutine instead) and future
conflict (with bug 19933).

Deal?

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #21 from M. Tompsett <[hidden email]> ---
(In reply to Jonathan Druart from comment #20)
> So actually we are trying to remove a warning because there are wrong calls
> in opac-main.
> The dashboard's info should be fetched only if a patron is logged in.
> I did this change on bug 12001 already.
> I am suggesting a new patch to avoid the change of the subroutine's
> prototype (which is wrong instead, but let remove the subroutine instead)
> and future conflict (with bug 19933).
>
> Deal?

While the problem that triggered this code change is that, and your simpler
change would suffice, I was hoping for a more thorough fix of the checkoverdues
function. Granted, this is only an intermediary step towards one that deals
with Koha::Checkouts filtered on is_overdue.

If you prefer, I could attempt to Koha::Checkouts the checkoverdues accessing
areas of code.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 19938] Refactor C4::Overdues::checkoverdues

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19938

--- Comment #22 from M. Tompsett <[hidden email]> ---
> (In reply to Jonathan Druart from comment #20)
> > The dashboard's info should be fetched only if a patron is logged in.

Re-read checkoverdue code...
The problem case is "or return".
Tweak to return (0,{}); perhaps?

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
12