https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17499 --- Comment #111 from Lari Taskula <lari.taskula@hypernova.fi> --- (In reply to Joonas Kylmälä from comment #110)
I see these message preferences as their own object tied to a patron (not to a patron's individual message). This would mean the name would be Koha::Patron::MessagePreference and Koha::Patron::MessagePreferences.
Good point. That makes sense. Let's go with that.
As far as I can tell the table message_attributes is incorrectly named / created, the columns should be in my opinion in the borrower_message_preferences table. ... Koha::Patron::MessagePreference (contains both borrower_message_preferences and message_attributes with JOIN)
This is basically what the current patches do, but without a join. The Message::Preference object has an accessor called "message_name" even though it's not physically part of borrower_message_preferences table. JOIN sounds interesting. But doesn't DBIx require you to use "table.column" key format when referencing to a column of a joined table? That's not very user friendly. Would it work with DBIx's "+as" option? Then we could get rid of the "find_with_message_name" method pointed out in comment #80 by Jonathan. I wonder if it's possible to define the JOIN in DBIx schema file so that we don't have to override search/find/whatever methods manually? While there could be improvement at database level, imo that's outside of this Bug's scope. At database level, I don't think they should be merged either. We need message_attribute for database normalization. borrower_message_preferences could also be improved by splitting it in two; borrower_message_preferences and category_message_preferences (the default category preferences). Anyways I think it's outside of the scope here.
The message_transports seems to contains also unrelated columns to transport types, namely the is_digest information which seems to be preference of the patron instead of something that is decided based on the transportation medium. But maybe you can fit this somehow nicely anyway? If it seems impossible we could restructure first the messaging preference related tables.
"message_options" would better describe its current data. It should also have a primary key integer. Then borrower_message_preferences columns "message_attribute_id" and "wants_digest" could be replaced with a foreign key to "message_option_id". So, borrower_message_preference would simply be a reference to a borrowernumber and message_option_id (and additionally having the days_in_advance setting). But your suggestion works as well. It's just a matter of selecting where the digest information should be at.
Also see comment #80 from Jonathan, I think it the $patron->messaging_preferences suggestion is a bit like what I have here but instead with an object.
We can implement the Koha::Patron accessors from comment #80, I've nothing against that. But $patron->messaging_preferences would be an object (of MessagingPreferences class), I'm not sure what you mean. -- You are receiving this mail because: You are watching all bug changes.