Skip to content

Commit fdc1a2e

Browse files
committed
Dropped role from Subscription
Removed `role` attribute from `Subscription` as it was providing no real additional value. All notifications are now sent **to** subscribers, and no longer copied. Changed some `debug` logging to be `info`.
1 parent 3415669 commit fdc1a2e

File tree

3 files changed

+30
-55
lines changed

3 files changed

+30
-55
lines changed

code_comments/db.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
'repos', 'rev'))[
2727
Column('id', auto_increment=True),
2828
Column('user'),
29-
Column('role'),
3029
Column('type'),
3130
Column('path'),
3231
Column('repos'),

code_comments/notification.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,17 @@ def get_recipients(self, comment):
5353
Current scheme is as follows:
5454
5555
* For the first comment in a given location, the notification is sent
56-
'to' the author of the resource being commented on, and 'copied'
57-
to any other subscribers to that resource
56+
to any subscribers to that resource
5857
* For any further comments in a given location, the notification is
59-
sent 'to' the author of the last comment in that location, and
60-
the author of the resource and 'copied' to any other subscribers
58+
sent to the author of the last comment in that location, and any other
59+
subscribers for that resource
6160
"""
6261
torcpts = set()
6362
ccrcpts = set()
6463

6564
for subscription in Subscription.for_comment(self.env, comment,
6665
notify=True):
67-
if subscription.role == 'author':
68-
torcpts.add(subscription.user)
69-
else:
70-
ccrcpts.add(subscription.user)
66+
torcpts.add(subscription.user)
7167

7268
# Is this a reply, or a new comment?
7369
thread = self._get_comment_thread(comment)

code_comments/subscription.py

Lines changed: 26 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ class Subscription(object):
1414
"""
1515
id = 0
1616
user = ''
17-
role = ''
1817
type = ''
1918
path = ''
2019
rev = ''
@@ -30,15 +29,15 @@ def __str__(self):
3029
"""
3130
Returns a user friendly string representation.
3231
"""
33-
template = "{0} as {1} for {2} {3}"
32+
template = "{0} for {1} {2}"
3433
if self.type == "changeset":
3534
_identifier = self.rev
3635
elif self.type == "browser":
3736
_identifier = "{0} @ {1}".format(self.path, self.rev)
3837
else:
3938
_identifier = self.path
4039

41-
return template.format(self.user, self.role, self.type, _identifier)
40+
return template.format(self.user, self.type, _identifier)
4241

4342
@classmethod
4443
def select(cls, env, args={}, notify=None):
@@ -79,10 +78,10 @@ def insert(self, db=None):
7978
def do_insert(db):
8079
cursor = db.cursor()
8180
insert = ("INSERT INTO code_comments_subscriptions "
82-
"(user, role, type, path, repos, rev, notify) "
83-
"VALUES (%s, %s, %s, %s, %s, %s, %s)")
84-
values = (self.user, self.role, self.type, self.path,
85-
self.repos, self.rev, self.notify)
81+
"(user, type, path, repos, rev, notify) "
82+
"VALUES (%s, %s, %s, %s, %s, %s)")
83+
values = (self.user, self.type, self.path, self.repos,
84+
self.rev, self.notify)
8685
cursor.execute(insert, values)
8786
self.id = db.get_last_id(cursor, 'code_comments_subscriptions')
8887
return True
@@ -99,10 +98,10 @@ def update(self, db=None):
9998
def do_update(db):
10099
cursor = db.cursor()
101100
update = ("UPDATE code_comments_subscriptions SET "
102-
"user=%s, role=%s, type=%s, path=%s, repos=%s, "
103-
"rev=%s, notify=%s WHERE id=%s")
104-
values = (self.user, self.role, self.type, self.path,
105-
self.repos, self.rev, self.notify, self.id)
101+
"user=%s, type=%s, path=%s, repos=%s, rev=%s, "
102+
"notify=%s WHERE id=%s")
103+
values = (self.user, self.type, self.path, self.repos,
104+
self.rev, self.notify, self.id)
106105
try:
107106
cursor.execute(update, values)
108107
except db.IntegrityError:
@@ -131,12 +130,11 @@ def _from_row(cls, env, row):
131130
subscription = cls(env)
132131
subscription.id = int(row[0])
133132
subscription.user = row[1]
134-
subscription.role = row[2]
135-
subscription.type = row[3]
136-
subscription.path = row[4]
137-
subscription.repos = row[5]
138-
subscription.rev = row[6]
139-
subscription.notify = bool(row[7])
133+
subscription.type = row[2]
134+
subscription.path = row[3]
135+
subscription.repos = row[4]
136+
subscription.rev = row[5]
137+
subscription.notify = bool(row[6])
140138
return subscription
141139
except IndexError:
142140
# Invalid row
@@ -163,26 +161,25 @@ def _from_dict(cls, env, dict_):
163161
for _subscription in subscriptions:
164162
if subscription is None:
165163
subscription = _subscription
166-
env.log.debug('Subscription already exists: [%d] %s',
167-
subscription.id, subscription)
164+
env.log.info('Subscription already exists: [%d] %s',
165+
subscription.id, subscription)
168166
else:
169167
# The unique constraint on the table should prevent this ever
170168
# occurring
171-
env.log.debug('Multiple subscriptions found: [%d] %s',
172-
subscription.id, subscription)
169+
env.log.warning('Multiple subscriptions found: [%d] %s',
170+
subscription.id, subscription)
173171

174172
# Create a new subscription if we didn't find one
175173
if subscription is None:
176174
subscription = cls(env, dict_)
177175
subscription.insert()
178-
env.log.debug('Subscription created: [%d] %s',
179-
subscription.id, subscription)
176+
env.log.info('Subscription created: [%d] %s',
177+
subscription.id, subscription)
180178

181179
return subscription
182180

183181
@classmethod
184-
def from_attachment(cls, env, attachment, user=None, role='author',
185-
notify=True):
182+
def from_attachment(cls, env, attachment, user=None, notify=True):
186183
"""
187184
Creates a subscription from an Attachment object.
188185
"""
@@ -192,7 +189,6 @@ def from_attachment(cls, env, attachment, user=None, role='author',
192189

193190
sub = {
194191
'user': user or attachment.author,
195-
'role': role,
196192
'type': 'attachment',
197193
'path': _path,
198194
'repos': '',
@@ -202,14 +198,12 @@ def from_attachment(cls, env, attachment, user=None, role='author',
202198
return cls._from_dict(env, sub)
203199

204200
@classmethod
205-
def from_changeset(cls, env, changeset, user=None, role='author',
206-
notify=True):
201+
def from_changeset(cls, env, changeset, user=None, notify=True):
207202
"""
208203
Creates a subscription from a Changeset object.
209204
"""
210205
sub = {
211206
'user': user or changeset.author,
212-
'role': role,
213207
'type': 'changeset',
214208
'path': '',
215209
'repos': changeset.repos.reponame,
@@ -219,14 +213,12 @@ def from_changeset(cls, env, changeset, user=None, role='author',
219213
return cls._from_dict(env, sub)
220214

221215
@classmethod
222-
def from_comment(cls, env, comment, user=None, role='commenter',
223-
notify=True):
216+
def from_comment(cls, env, comment, user=None, notify=True):
224217
"""
225218
Creates a subscription from a Comment object.
226219
"""
227220
sub = {
228221
'user': user or comment.author,
229-
'role': 'commenter',
230222
'type': comment.type,
231223
'notify': notify,
232224
}
@@ -388,21 +380,9 @@ def changeset_added(self, repos, changeset):
388380
Subscription.from_changeset(self.env, changeset)
389381

390382
def changeset_modified(self, repos, changeset, old_changeset):
391-
# Handle existing subscriptions
392383
if changeset.author != old_changeset.author:
393-
for subscription in Subscription.for_changeset(self.env,
394-
changeset):
395-
if (subscription.user == old_changeset.author and
396-
subscription.role == 'author'):
397-
subscription.role = 'subscriber'
398-
399-
if subscription.user == changeset.author:
400-
subscription.role = 'author'
401-
402-
subscription.update()
403-
404-
# Create a new author subscription
405-
Subscription.from_changeset(self.env, changeset)
384+
# Create a new author subscription
385+
Subscription.from_changeset(self.env, changeset)
406386

407387
# ICodeCommentChangeListener methods
408388

0 commit comments

Comments
 (0)