Comment 4 for bug 677074

Revision history for this message
Mike Rylander (mrylander) wrote :

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.