tangled.org
Signed-off-by: Seongmin Lee git@boltless.me
+125
-144
Diff
round #0
+1
-1
appview/issues/issues.go
+1
-1
appview/issues/issues.go
···
488
// reset atUri to make rollback a no-op
489
atUri = ""
490
491
-
rp.notifier.NewIssueComment(r.Context(), &comment, mentions)
492
493
ownerSlashRepo := reporesolver.GetBaseRepoPath(r, f)
494
rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d#comment-%d", ownerSlashRepo, issue.IssueId, comment.Id))
+105
-110
appview/notify/db/db.go
+105
-110
appview/notify/db/db.go
···
73
// no-op
74
}
75
76
-
func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {
77
78
-
// build the recipients list
79
-
// - owner of the repo
80
-
// - collaborators in the repo
81
-
var recipients []syntax.DID
82
-
recipients = append(recipients, syntax.DID(issue.Repo.Did))
83
-
collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", issue.Repo.RepoAt()))
84
if err != nil {
85
-
log.Printf("failed to fetch collaborators: %v", err)
86
return
87
}
88
-
for _, c := range collaborators {
89
-
recipients = append(recipients, c.SubjectDid)
90
-
}
91
92
-
actorDid := syntax.DID(issue.Did)
93
-
entityType := "issue"
94
-
entityId := issue.AtUri().String()
95
-
repoId := &issue.Repo.Id
96
-
issueId := &issue.Id
97
-
var pullId *int64
98
99
n.notifyEvent(
100
-
actorDid,
101
recipients,
102
-
models.NotificationTypeIssueCreated,
103
entityType,
104
entityId,
105
repoId,
···
107
pullId,
108
)
109
n.notifyEvent(
110
-
actorDid,
111
-
mentions,
112
models.NotificationTypeUserMentioned,
113
entityType,
114
entityId,
···
118
)
119
}
120
121
-
func (n *databaseNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
122
-
issues, err := db.GetIssues(n.db, db.FilterEq("at_uri", comment.Subject))
123
-
if err != nil {
124
-
log.Printf("NewIssueComment: failed to get issues: %v", err)
125
-
return
126
-
}
127
-
if len(issues) == 0 {
128
-
log.Printf("NewIssueComment: no issue found for %s", comment.Subject)
129
-
return
130
-
}
131
-
issue := issues[0]
132
133
var recipients []syntax.DID
134
recipients = append(recipients, syntax.DID(issue.Repo.Did))
135
-
136
-
if comment.IsReply() {
137
-
// if this comment is a reply, then notify everybody in that thread
138
-
parentAtUri := *comment.ReplyTo
139
-
allThreads := issue.CommentList()
140
-
141
-
// find the parent thread, and add all DIDs from here to the recipient list
142
-
for _, t := range allThreads {
143
-
if t.Self.AtUri() == parentAtUri {
144
-
recipients = append(recipients, t.Participants()...)
145
-
}
146
-
}
147
-
} else {
148
-
// not a reply, notify just the issue author
149
-
recipients = append(recipients, syntax.DID(issue.Did))
150
}
151
152
-
actorDid := syntax.DID(comment.Did)
153
entityType := "issue"
154
entityId := issue.AtUri().String()
155
repoId := &issue.Repo.Id
···
159
n.notifyEvent(
160
actorDid,
161
recipients,
162
-
models.NotificationTypeIssueCommented,
163
entityType,
164
entityId,
165
repoId,
···
248
)
249
}
250
251
-
func (n *databaseNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
252
-
pulls, err := db.GetPulls(n.db,
253
-
db.FilterEq("owner_did", comment.Subject.Authority()),
254
-
db.FilterEq("rkey", comment.Subject.RecordKey()),
255
-
)
256
-
if err != nil {
257
-
log.Printf("NewPullComment: failed to get pulls: %v", err)
258
-
return
259
-
}
260
-
if len(pulls) == 0 {
261
-
log.Printf("NewPullComment: no pull found for %s", comment.Subject)
262
-
return
263
-
}
264
-
pull := pulls[0]
265
-
266
-
repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", pull.RepoAt))
267
-
if err != nil {
268
-
log.Printf("NewPullComment: failed to get repos: %v", err)
269
-
return
270
-
}
271
-
272
-
// build up the recipients list:
273
-
// - repo owner
274
-
// - all pull participants
275
-
var recipients []syntax.DID
276
-
recipients = append(recipients, syntax.DID(repo.Did))
277
-
for _, p := range pull.Participants() {
278
-
recipients = append(recipients, syntax.DID(p))
279
-
}
280
-
281
-
actorDid := comment.Did
282
-
eventType := models.NotificationTypePullCommented
283
-
entityType := "pull"
284
-
entityId := pull.AtUri().String()
285
-
repoId := &repo.Id
286
-
var issueId *int64
287
-
p := int64(pull.ID)
288
-
pullId := &p
289
-
290
-
n.notifyEvent(
291
-
actorDid,
292
-
recipients,
293
-
eventType,
294
-
entityType,
295
-
entityId,
296
-
repoId,
297
-
issueId,
298
-
pullId,
299
-
)
300
-
n.notifyEvent(
301
-
actorDid,
302
-
mentions,
303
-
models.NotificationTypeUserMentioned,
304
-
entityType,
305
-
entityId,
306
-
repoId,
307
-
issueId,
308
-
pullId,
309
-
)
310
-
}
311
-
312
func (n *databaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {
313
// no-op
314
}
···
73
// no-op
74
}
75
76
+
func (n *databaseNotifier) NewComment(ctx context.Context, comment *models.Comment) {
77
+
var (
78
+
recipients []syntax.DID
79
+
entityType string
80
+
entityId string
81
+
repoId *int64
82
+
issueId *int64
83
+
pullId *int64
84
+
)
85
86
+
subjectDid, err := comment.Subject.Authority().AsDID()
87
if err != nil {
88
+
log.Printf("NewComment: expected did based at-uri for comment.subject")
89
return
90
}
91
+
switch comment.Subject.Collection() {
92
+
case tangled.RepoIssueNSID:
93
+
issues, err := db.GetIssues(
94
+
n.db,
95
+
db.FilterEq("did", subjectDid),
96
+
db.FilterEq("rkey", comment.Subject.RecordKey()),
97
+
)
98
+
if err != nil {
99
+
log.Printf("NewComment: failed to get issues: %v", err)
100
+
return
101
+
}
102
+
if len(issues) == 0 {
103
+
log.Printf("NewComment: no issue found for %s", comment.Subject)
104
+
return
105
+
}
106
+
issue := issues[0]
107
+
108
+
recipients = append(recipients, syntax.DID(issue.Repo.Did))
109
+
if comment.IsReply() {
110
+
// if this comment is a reply, then notify everybody in that thread
111
+
parentAtUri := *comment.ReplyTo
112
+
allThreads := issue.CommentList()
113
+
114
+
// find the parent thread, and add all DIDs from here to the recipient list
115
+
for _, t := range allThreads {
116
+
if t.Self.AtUri() == parentAtUri {
117
+
recipients = append(recipients, t.Participants()...)
118
+
}
119
+
}
120
+
} else {
121
+
// not a reply, notify just the issue author
122
+
recipients = append(recipients, syntax.DID(issue.Did))
123
+
}
124
125
+
entityType = "issue"
126
+
entityId = issue.AtUri().String()
127
+
repoId = &issue.Repo.Id
128
+
issueId = &issue.Id
129
+
case tangled.RepoPullNSID:
130
+
pulls, err := db.GetPullsWithLimit(
131
+
n.db,
132
+
1,
133
+
db.FilterEq("owner_did", subjectDid),
134
+
db.FilterEq("rkey", comment.Subject.RecordKey()),
135
+
)
136
+
if err != nil {
137
+
log.Printf("NewComment: failed to get pulls: %v", err)
138
+
return
139
+
}
140
+
if len(pulls) == 0 {
141
+
log.Printf("NewComment: no pull found for %s", comment.Subject)
142
+
return
143
+
}
144
+
pull := pulls[0]
145
+
146
+
pull.Repo, err = db.GetRepo(n.db, db.FilterEq("at_uri", pull.RepoAt))
147
+
if err != nil {
148
+
log.Printf("NewComment: failed to get repos: %v", err)
149
+
return
150
+
}
151
+
152
+
recipients = append(recipients, syntax.DID(pull.Repo.Did))
153
+
for _, p := range pull.Participants() {
154
+
recipients = append(recipients, syntax.DID(p))
155
+
}
156
+
157
+
entityType = "pull"
158
+
entityId = pull.AtUri().String()
159
+
repoId = &pull.Repo.Id
160
+
p := int64(pull.ID)
161
+
pullId = &p
162
+
default:
163
+
return // no-op
164
+
}
165
166
n.notifyEvent(
167
+
comment.Did,
168
recipients,
169
+
models.NotificationTypeIssueCommented,
170
entityType,
171
entityId,
172
repoId,
···
174
pullId,
175
)
176
n.notifyEvent(
177
+
comment.Did,
178
+
comment.Mentions,
179
models.NotificationTypeUserMentioned,
180
entityType,
181
entityId,
···
185
)
186
}
187
188
+
func (n *databaseNotifier) DeleteComment(ctx context.Context, comment *models.Comment) {
189
+
// no-op
190
+
}
191
+
192
+
func (n *databaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {
193
194
+
// build the recipients list
195
+
// - owner of the repo
196
+
// - collaborators in the repo
197
var recipients []syntax.DID
198
recipients = append(recipients, syntax.DID(issue.Repo.Did))
199
+
collaborators, err := db.GetCollaborators(n.db, db.FilterEq("repo_at", issue.Repo.RepoAt()))
200
+
if err != nil {
201
+
log.Printf("failed to fetch collaborators: %v", err)
202
+
return
203
+
}
204
+
for _, c := range collaborators {
205
+
recipients = append(recipients, c.SubjectDid)
206
}
207
208
+
actorDid := syntax.DID(issue.Did)
209
entityType := "issue"
210
entityId := issue.AtUri().String()
211
repoId := &issue.Repo.Id
···
215
n.notifyEvent(
216
actorDid,
217
recipients,
218
+
models.NotificationTypeIssueCreated,
219
entityType,
220
entityId,
221
repoId,
···
304
)
305
}
306
307
func (n *databaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {
308
// no-op
309
}
+8
-8
appview/notify/merged_notifier.go
+8
-8
appview/notify/merged_notifier.go
···
54
m.fanout("DeleteStar", ctx, star)
55
}
56
57
-
func (m *mergedNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {
58
-
m.fanout("NewIssue", ctx, issue, mentions)
59
}
60
61
-
func (m *mergedNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
62
-
m.fanout("NewIssueComment", ctx, comment, mentions)
63
}
64
65
func (m *mergedNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) {
···
82
m.fanout("NewPull", ctx, pull)
83
}
84
85
-
func (m *mergedNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
86
-
m.fanout("NewPullComment", ctx, comment, mentions)
87
-
}
88
-
89
func (m *mergedNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {
90
m.fanout("NewPullState", ctx, actor, pull)
91
}
···
54
m.fanout("DeleteStar", ctx, star)
55
}
56
57
+
func (m *mergedNotifier) NewComment(ctx context.Context, comment *models.Comment) {
58
+
m.fanout("NewComment", ctx, comment)
59
+
}
60
+
61
+
func (m *mergedNotifier) DeleteComment(ctx context.Context, comment *models.Comment) {
62
+
m.fanout("DeleteComment", ctx, comment)
63
}
64
65
+
func (m *mergedNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {
66
+
m.fanout("NewIssue", ctx, issue, mentions)
67
}
68
69
func (m *mergedNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) {
···
86
m.fanout("NewPull", ctx, pull)
87
}
88
89
func (m *mergedNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {
90
m.fanout("NewPullState", ctx, actor, pull)
91
}
+6
-6
appview/notify/notifier.go
+6
-6
appview/notify/notifier.go
···
13
NewStar(ctx context.Context, star *models.Star)
14
DeleteStar(ctx context.Context, star *models.Star)
15
16
NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID)
17
-
NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID)
18
NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue)
19
DeleteIssue(ctx context.Context, issue *models.Issue)
20
···
22
DeleteFollow(ctx context.Context, follow *models.Follow)
23
24
NewPull(ctx context.Context, pull *models.Pull)
25
-
NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID)
26
NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull)
27
28
UpdateProfile(ctx context.Context, profile *models.Profile)
···
42
func (m *BaseNotifier) NewStar(ctx context.Context, star *models.Star) {}
43
func (m *BaseNotifier) DeleteStar(ctx context.Context, star *models.Star) {}
44
45
func (m *BaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {}
46
-
func (m *BaseNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
47
-
}
48
func (m *BaseNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) {}
49
func (m *BaseNotifier) DeleteIssue(ctx context.Context, issue *models.Issue) {}
50
···
52
func (m *BaseNotifier) DeleteFollow(ctx context.Context, follow *models.Follow) {}
53
54
func (m *BaseNotifier) NewPull(ctx context.Context, pull *models.Pull) {}
55
-
func (m *BaseNotifier) NewPullComment(ctx context.Context, models *models.Comment, mentions []syntax.DID) {
56
-
}
57
func (m *BaseNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {}
58
59
func (m *BaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {}
···
13
NewStar(ctx context.Context, star *models.Star)
14
DeleteStar(ctx context.Context, star *models.Star)
15
16
+
NewComment(ctx context.Context, comment *models.Comment)
17
+
DeleteComment(ctx context.Context, comment *models.Comment)
18
+
19
NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID)
20
NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue)
21
DeleteIssue(ctx context.Context, issue *models.Issue)
22
···
24
DeleteFollow(ctx context.Context, follow *models.Follow)
25
26
NewPull(ctx context.Context, pull *models.Pull)
27
NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull)
28
29
UpdateProfile(ctx context.Context, profile *models.Profile)
···
43
func (m *BaseNotifier) NewStar(ctx context.Context, star *models.Star) {}
44
func (m *BaseNotifier) DeleteStar(ctx context.Context, star *models.Star) {}
45
46
+
func (m *BaseNotifier) NewComment(ctx context.Context, comment *models.Comment) {}
47
+
func (m *BaseNotifier) DeleteComment(ctx context.Context, comment *models.Comment) {}
48
+
49
func (m *BaseNotifier) NewIssue(ctx context.Context, issue *models.Issue, mentions []syntax.DID) {}
50
func (m *BaseNotifier) NewIssueState(ctx context.Context, actor syntax.DID, issue *models.Issue) {}
51
func (m *BaseNotifier) DeleteIssue(ctx context.Context, issue *models.Issue) {}
52
···
54
func (m *BaseNotifier) DeleteFollow(ctx context.Context, follow *models.Follow) {}
55
56
func (m *BaseNotifier) NewPull(ctx context.Context, pull *models.Pull) {}
57
func (m *BaseNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {}
58
59
func (m *BaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {}
+4
-18
appview/notify/posthog/notifier.go
+4
-18
appview/notify/posthog/notifier.go
···
86
}
87
}
88
89
-
func (n *posthogNotifier) NewPullComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
90
-
err := n.client.Enqueue(posthog.Capture{
91
-
DistinctId: comment.Did.String(),
92
-
Event: "new_pull_comment",
93
-
Properties: posthog.Properties{
94
-
"pull_at": comment.Subject,
95
-
"mentions": mentions,
96
-
},
97
-
})
98
-
if err != nil {
99
-
log.Println("failed to enqueue posthog event:", err)
100
-
}
101
-
}
102
-
103
func (n *posthogNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) {
104
err := n.client.Enqueue(posthog.Capture{
105
DistinctId: pull.OwnerDid,
···
179
}
180
}
181
182
-
func (n *posthogNotifier) NewIssueComment(ctx context.Context, comment *models.Comment, mentions []syntax.DID) {
183
err := n.client.Enqueue(posthog.Capture{
184
DistinctId: comment.Did.String(),
185
-
Event: "new_issue_comment",
186
Properties: posthog.Properties{
187
-
"issue_at": comment.Subject,
188
-
"mentions": mentions,
189
},
190
})
191
if err != nil {
···
86
}
87
}
88
89
func (n *posthogNotifier) NewPullClosed(ctx context.Context, pull *models.Pull) {
90
err := n.client.Enqueue(posthog.Capture{
91
DistinctId: pull.OwnerDid,
···
165
}
166
}
167
168
+
func (n *posthogNotifier) NewComment(ctx context.Context, comment *models.Comment) {
169
err := n.client.Enqueue(posthog.Capture{
170
DistinctId: comment.Did.String(),
171
+
Event: "new_comment",
172
Properties: posthog.Properties{
173
+
"subject_at": comment.Subject,
174
+
"mentions": comment.Mentions,
175
},
176
})
177
if err != nil {
+1
-1
appview/pulls/pulls.go
+1
-1
appview/pulls/pulls.go
History
8 rounds
6 comments
boltless.me
submitted
#7
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
2/3 failed, 1/3 success
expand
collapse
merge conflicts detected
expand
collapse
expand
collapse
- appview/notify/db/db.go:260
- appview/notify/merged_notifier.go:81
- appview/notify/notifier.go:22
- appview/pulls/opengraph.go:277
expand 0 comments
boltless.me
submitted
#6
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
1/3 failed, 2/3 success
expand
collapse
expand 0 comments
boltless.me
submitted
#5
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
3/3 success
expand
collapse
expand 0 comments
boltless.me
submitted
#4
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
3/3 success
expand
collapse
expand 2 comments
@oppi.li thank you for the review!
when we intend to perform backfill of records, we should support ingesting the old records.
Yeah I think it would be better to leave ingester logic for issue.comment update/delete operations. Users should be able to modify existing records.
Backfilling issue.comment will be quite tricky as we cannot distinguish new comments since certain timestamp. Users can modify both created field and rkey.
So there are only two options:
- ingest legacy comment records until we completely drop them (we should silently migrate user records as much as possible before that)
- have "legacy records" list in tangled appview and only ingest for those records. This way, other appviews won't be able to support legacy records.
we can stop maintaining the old record for much longer, although i suspect that the new comment record will be quite stable
There are few issues that can threat the comment lexicon stability:
I think we are roughly fine after changing body field type to sh.tangled.markup#markdown. Though we might we need more discussion for #328 just in case.
boltless.me
submitted
#3
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
1/3 failed, 2/3 success
expand
collapse
expand 0 comments
boltless.me
submitted
#2
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
3/3 success
expand
collapse
expand 0 comments
boltless.me
submitted
#1
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
3/3 success
expand
collapse
boltless.me
submitted
#0
1 commit
expand
collapse
appview/notify: merge new comment events into one
Signed-off-by: Seongmin Lee <git@boltless.me>
1/3 failed, 2/3 timeout
expand
collapse
expand 3 comments
note: I'm choosing sh.tangled.comment NSID here. Tell me if we would prefer something else like sh.tangled.feed.comment
I think sh.tangled.comment is fine, but more importantly, are we still sticking to sh.tangled or does it make sense to move this to the org.tangled namespace? Assuming we're moving everything eventually. cc @oppi.li
I think we should stick on sh.tangled for now as there can be large amounts of changes from PR refactor / did-for-repo. We can consider moving entire collections to org.tangled once tangled lexicons become more stable.
on the whole this changeset lgtm. some questions i have regarding backwards compatibility:
sh.tangled.issue.commentwill not be ingested anymore