diff --git a/fmn/lib/models.py b/fmn/lib/models.py index 5e690b0..c03fc97 100644 --- a/fmn/lib/models.py +++ b/fmn/lib/models.py @@ -141,16 +141,22 @@ def create(cls, session, name, description, def _recipients(self, session, config, valid_paths, message): """ Returns the list of recipients for a message. """ + for user in User.all(session): pref = Preference.load(session, user, self) - if pref and pref.detail_value: - filter = pref.prefers(session, config, valid_paths, message) - if filter: - yield { - 'user': user.openid, - pref.context.detail_name: pref.detail_value.split(','), - 'filter': filter.name, - } + if not pref or not pref.detail_values: + continue + + flter = pref.prefers(session, config, valid_paths, message) + if not flter: + continue + + for value in pref.detail_values: + yield { + 'user': user.openid, + pref.context.detail_name: value.value, + 'filter': flter.name, + } def recipients(self, session, config, valid_paths, message): return list(self._recipients(session, config, valid_paths, message)) @@ -189,7 +195,11 @@ def get_or_create(cls, session, openid, openid_url, create_defaults=True): @classmethod def create(cls, session, openid, openid_url, create_defaults): - user = cls(openid=openid, openid_url=openid_url, api_key=str(uuid.uuid4())) + user = cls( + openid=openid, + openid_url=openid_url, + api_key=str(uuid.uuid4()), + ) session.add(user) session.flush() @@ -348,13 +358,43 @@ def matches(self, session, config, paths, message): return True +class DetailValue(BASE): + __tablename__ = 'detail_values' + id = sa.Column(sa.Integer, primary_key=True) + created_on = sa.Column(sa.DateTime, default=datetime.datetime.utcnow) + value = sa.Column(sa.String(1024), unique=True) + preference_id = sa.Column( + sa.Integer, + sa.ForeignKey('preferences.id')) + preference = relation('Preference', backref=backref('detail_values')) + + @classmethod + def get(cls, session, value): + return session.query(cls).filter(cls.value==value).first() + + @classmethod + def create(cls, session, value): + obj = cls() + obj.value = value + session.add(obj) + session.commit() + return obj + + @classmethod + def exists(cls, session, value): + return ( + session.query(cls).filter( + cls.value == value).count() > 0 or + session.query(Confirmation).filter( + Confirmation.detail_value == value).count() > 0 + ) + + class Preference(BASE): __tablename__ = 'preferences' id = sa.Column(sa.Integer, primary_key=True) created_on = sa.Column(sa.DateTime, default=datetime.datetime.utcnow) - detail_value = sa.Column(sa.String(1024), unique=True) - # Number of seconds that have elapsed since the earliest queued message # before we send a digest over whatever medium. batch_delta = sa.Column(sa.Integer, nullable=True) @@ -403,7 +443,7 @@ def set_batch_values(self, session, delta, count): session.commit() @classmethod - def by_user(cls, session, openid, allow_none=False): + def by_user(cls, session, openid): query = session.query( cls ).filter( @@ -412,18 +452,12 @@ def by_user(cls, session, openid, allow_none=False): cls.context_name ) - if not allow_none: - query = query.filter(sa.not_(cls.detail_value == None)) - return query.all() @classmethod def by_detail(cls, session, detail_value): - return session.query( - cls - ).filter( - cls.detail_value == detail_value - ).first() + value = DetailValue.get(session, detail_value) + return value.preference @classmethod def create(cls, session, user, context, detail_value=None): @@ -435,7 +469,11 @@ def create(cls, session, user, context, detail_value=None): pref.user = user pref.context = context - pref.detail_value = detail_value + if detail_value: + value = DetailValue.create(session, detail_value) + pref.detail_values.append(value) + + session.add(value) session.add(pref) session.flush() @@ -471,14 +509,10 @@ def load(cls, session, user, context): .filter_by(context_name=context)\ .first() - def update_details(self, session, value): - value = value.strip() - if self.detail_value: - tokens = self.detail_value.split(',') - if value not in tokens: - self.detail_value = ','.join(tokens + [value]) - else: - self.detail_value = value + def update_details(self, session, detail_value): + detail_value = detail_value.strip() + value = DetailValue.create(session, detail_value) + self.detail_values.append(value) session.flush() session.commit() diff --git a/fmn/lib/tests/test_confirmations.py b/fmn/lib/tests/test_confirmations.py index 51b993f..43b6984 100644 --- a/fmn/lib/tests/test_confirmations.py +++ b/fmn/lib/tests/test_confirmations.py @@ -35,11 +35,20 @@ def test_updating_details(self): context = fmn.lib.models.Context.get(self.sess, name="irc") preference = fmn.lib.models.Preference.load(self.sess, user, context) preference.update_details(self.sess, 'wat') - eq_(preference.detail_value, 'wat') + eq_(preference.detail_values[0].value, 'wat') preference.update_details(self.sess, 'wat2') - eq_(preference.detail_value, 'wat,wat2') - preference.update_details(self.sess, 'wat') - eq_(preference.detail_value, 'wat,wat2') + eq_(preference.detail_values[0].value, 'wat') + eq_(preference.detail_values[1].value, 'wat2') + + try: + preference.update_details(self.sess, 'wat') + assert(False) + except Exception: + self.sess.rollback() + + eq_(len(preference.detail_values), 2) + eq_(preference.detail_values[0].value, 'wat') + eq_(preference.detail_values[1].value, 'wat2') def test_confirmation(self): self.create_user_and_context_data() @@ -54,6 +63,6 @@ def test_confirmation(self): context, detail_value='awesome', ) - eq_(preference.detail_value, None) + eq_(preference.detail_values, []) confirmation.set_status(self.sess, 'accepted') - eq_(preference.detail_value, 'awesome') + eq_(preference.detail_values[0].value, 'awesome') diff --git a/fmn/lib/tests/test_recipients.py b/fmn/lib/tests/test_recipients.py index 960bc20..9274837 100644 --- a/fmn/lib/tests/test_recipients.py +++ b/fmn/lib/tests/test_recipients.py @@ -32,7 +32,7 @@ def create_preference_data_empty(self): self.sess, user=user, context=context, - detail_value="threebean,threebean2", + detail_value="threebean", ) def create_preference_data_basic(self, code_path): @@ -79,7 +79,7 @@ def test_basic_recipients_list(self): recipients = fmn.lib.recipients_for_context( self.sess, self.config, self.valid_paths, 'irc', msg) eq_(list(recipients), [{ - 'irc nick': ['threebean', 'threebean2'], + 'irc nick': 'threebean', 'user': 'ralph.id.fedoraproject.org', 'filter': 'test filter', }]) @@ -139,7 +139,7 @@ def test_multiple_identical_filters_hit(self): recipients = fmn.lib.recipients_for_context( self.sess, self.config, self.valid_paths, 'irc', msg) eq_(list(recipients), [{ - 'irc nick': ['threebean', 'threebean2'], + 'irc nick': 'threebean', 'user': 'ralph.id.fedoraproject.org', 'filter': 'test filter', }]) @@ -164,7 +164,7 @@ def test_multiple_different_filters_hit(self): recipients = fmn.lib.recipients_for_context( self.sess, self.config, self.valid_paths, 'irc', msg) eq_(list(recipients), [{ - 'irc nick': ['threebean', 'threebean2'], + 'irc nick': 'threebean', 'user': 'ralph.id.fedoraproject.org', 'filter': 'test filter', }])