Evergreen

check constraint valid_fine_check in asset.copy_template DDL statement defined incorrectly at line 497 of 040.schema.asset.sql

Bug #1384796 reported by Tim Faile
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Confirmed
Medium
Unassigned

Bug Description

It appears that the DDL (CREATE TABLE) statement wrongly points to loan_duration, rather than to fine_level when it checks for valid values 1, 2, or 3 in the "valid_fine_level" check constraint at line 497 of 040.schema.asset.sql. Not sure what is intended in that check constraint, but it would seem logical to have the "valid_fine_level" check actually check values set in "fine_level", or else this will (and in fact probably does) act as a table-level (rather than column-level) constraint.

I do not know how this may affect functionality in Evergreen (if it does at all) since it was detected directly in the Evergreen source code (current version 2.6 and as early as 2.4--maybe earlier as well) and since it (in part) repeats the CHECK constraint for loan_duration values, but if confirmed as a bug, it should just be corrected.

At the database level (at least), the worst that could happen is that this CHECK constraint would allow any INT value assigned to fine_level column when creating/editing asset.copy_template records. I have not confirmed this behavior.

A simple fix (see the attached patch file) would be to replace line 497:

                                    fine_level IS NULL OR loan_duration IN (1,2,3)),

with this line:

                                    fine_level IS NULL OR fine_level IN (1,2,3)),

In that recommended change, I did not alter the accepted values part. I based the accepted values for fine_level on that column's CHECK constraint defined in the asset.copy CREATE TABLE statement in the same file (line 83), as that seemed to make sense. Hope this helps!

Revision history for this message
Tim Faile (tfaile-g) wrote :
Revision history for this message
Ben Shum (bshum) wrote :

Hi Tim,

Interesting catch! I can totally see this being something nice to have fixed in a future release for clarity.

If you can provide us with some more information on your contribution, specifically a DCO -- your name, organization, email for contribution purposes, that'd be helpful! (http://wiki.evergreen-ils.org/doku.php?id=contributing#developer_s_certificate_of_origin)

Thanks!

Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Tim Faile (tfaile-g) wrote :

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Signed-off-by:

Tim Faile
Library Data & Support Specialist
Equinox Software, Inc.
<email address hidden>

Revision history for this message
Tim Faile (tfaile-g) wrote :

Hi Ben,

Let me know if you need anything else.

Regards,

Tim

Jason Etheridge (phasefx)
Changed in evergreen:
assignee: nobody → Jason Etheridge (phasefx)
Jason Etheridge (phasefx)
Changed in evergreen:
assignee: Jason Etheridge (phasefx) → nobody
Revision history for this message
Andrea Neiman (aneiman) wrote :

This is probably fixedinwebby, but someone will need to look at the code to be sure

tags: added: copytemplates
removed: copy template
Andrea Neiman (aneiman)
tags: added: itemtemplates
removed: copytemplates
Beth Willis (willis-a)
tags: added: cat-templates
removed: itemtemplates
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.