This is an initial review, focusing on obvious (to me) issues. Not final, by any means, just intended to move things along. I'm working from Open-ILS/src/sql/Pg/upgrade/0XXX.config.matrix_weights.sql, which should include the full changes needed to apply the patch to a live db.
* permission.grp_ancestors_distance and friends should use the prevailing "explicit JOIN" style instead of WHERE-clause joins in order to match the general style for maintenance purposes.
* Need an fkey on config.weight_assoc.org_unit (perhaps it should be called "owner"?) and all fkeys on this table should have cascade clauses -- CASCADE on org_unit and SET NULL on the weights links.
* action.find_circ_matrix_matchpoint and action.find_hold_matrix_matchpoint
- Instead of using COALESCE to supply the default weighting, perhaps just set the fields of the "weights" variable if no weighting is found for the context_ou. Should be simpler (or as simple) to maintain, and removes the cost of many COALESCEs.
- Apply a LIMIT to the "Grab the closest set circ weight setting." Not strictly necessary, but more self-documenting
* Should probably go ahead and surface the user_home_ou bits in this patch, since refactoring it later seems, now, like make-work, when it's clear that users want it.
So mostly just style cleanup. The wait seems anti-climatic now, eh?
If you'll polish those bits I will go ahead and commit to trunk.
This is an initial review, focusing on obvious (to me) issues. Not final, by any means, just intended to move things along. I'm working from Open-ILS/ src/sql/ Pg/upgrade/ 0XXX.config. matrix_ weights. sql, which should include the full changes needed to apply the patch to a live db.
* permission. grp_ancestors_ distance and friends should use the prevailing "explicit JOIN" style instead of WHERE-clause joins in order to match the general style for maintenance purposes.
* Need an fkey on config. weight_ assoc.org_ unit (perhaps it should be called "owner"?) and all fkeys on this table should have cascade clauses -- CASCADE on org_unit and SET NULL on the weights links.
* action. find_circ_ matrix_ matchpoint and action. find_hold_ matrix_ matchpoint
- Instead of using COALESCE to supply the default weighting, perhaps just set the fields of the "weights" variable if no weighting is found for the context_ou. Should be simpler (or as simple) to maintain, and removes the cost of many COALESCEs.
- Apply a LIMIT to the "Grab the closest set circ weight setting." Not strictly necessary, but more self-documenting
* Should probably go ahead and surface the user_home_ou bits in this patch, since refactoring it later seems, now, like make-work, when it's clear that users want it.
So mostly just style cleanup. The wait seems anti-climatic now, eh?
If you'll polish those bits I will go ahead and commit to trunk.