Summary: | Rename reserves.item_level_hold for reserves.item_level | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | RESOLVED DUPLICATE | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 9834 | ||
Bug Blocks: | |||
Attachments: |
Bug 23133: DB update
Bug 23133: Rename uses of item_level_hold Bug 23133: API cleanup Bug 23133: DB update Bug 23133: Rename uses of item_level_hold Bug 23133: API cleanup |
Description
Tomás Cohen Arazi (tcohen)
2019-06-17 15:22:23 UTC
Created attachment 90679 [details] [review] Bug 23133: DB update Created attachment 90680 [details] [review] Bug 23133: Rename uses of item_level_hold Created attachment 90681 [details] [review] Bug 23133: API cleanup Ok, my two cents. I included the schema updates, let me know if you need it separate. suspend in the same table is still BOOLEAN. If SQL12 is applied we should patch all existing occurrences, and have a check in the QA script. Is there a bug report already? (In reply to Jonathan Druart from comment #5) > suspend in the same table is still BOOLEAN. BOOLEAN is just an alias for TINYINT(1), I think it is worth changing anyway unless... > If SQL12 is applied we should patch all existing occurrences, and have a > check in the QA script. Is there a bug report already? we have this kind of thing, and we actually prefer to add them as BOOLEAN in kohastructure.sql, because we have TINYINT(1) fields that are not really booleans (for example marc_subfield_structure.tab). There isn't a bug report yet (sorry for that), and it would be great to have the QA tools catch this problems. If BOOLEAN helps detect them, then lets change the rule and use BOOLEAN! Moving boolean discussion to: https://gitlab.com/koha-community/qa-test-tools/issues/20 Sorry mate, but I can't apply this one.. error: sha1 information is lacking or useless (Koha/Schema/Result/Reserve.pm). error: could not build fake ancestor Patch failed at 0001 Bug 23133: DB update That'll be the schema updates.. I'll work around it. Created attachment 90857 [details] [review] Bug 23133: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 90858 [details] [review] Bug 23133: Rename uses of item_level_hold Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 90859 [details] [review] Bug 23133: API cleanup Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I'm happy with the changes as they are here, but do agree it would be nice to see the BOOLEAN handling and QA handling of such cases dealt with.. lets get this one rolling whilst we discus and implement an approach to resolving those issues in the gitlab bug linked above and bug 21294 here. Koha/Schema/Result/OldReserve.pm | 8 ++++---- Koha/Schema/Result/Reserve.pm | 8 ++++---- .../atomicupdate/bug_23133_item_level_holds.perl | 19 +++++++++++++++++++ installer/data/mysql/kohastructure.sql | 4 ++-- I would not recommend doing this in one patch. And thought that we even had a rule for that? But can't find it. DBIx schema files should be separate and marked as do_not_push, if the RM wants to generate them himself. I had errors generating the updated schema with the is_boolean declaration. That's why I added it like this. |