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

[Tim Faile]

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!

[Ben Shum (bshum)]

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!

[Tim Faile (tfaile-g)]

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>

[Tim Faile (tfaile-g)]

Hi Ben,

Let me know if you need anything else.

Regards,

Tim

[Andrea Neiman (aneiman)]

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

[Jason Stephenson (jstephenson)]

Looks like this patch is still relevant as serials uses the asset.copy_template table in recent releases of Evergreen.

I have pushed a branch based on Tim's patch with a database upgrade and miscellaneous release note to the working repository: user/dyrcona/lp1384796-copy-template-constraint-fix

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1384796-copy-template-constraint-fix

[Jane Sandberg (sandbergja)]

Thanks, Tim and Jason. Signoff here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1384796-copy-template-constraint-fix-signoff

[Galen Charlton (gmc)]

Pushed down to rel_3_11. Thanks, Tim, Jason, and Jane!