Monorepo for Tangled
appview: fix issue/pr notifications being skipped
Lewis: May this revision serve well! <lewis@tangled.org>
+324
-5
+4
-1
appview/db/collaborators.go
+4
-1
appview/db/collaborators.go
···
11
11
12
12
func AddCollaborator(e Execer, c models.Collaborator) error {
13
13
_, err := e.Exec(
14
-
`insert into collaborators (did, rkey, subject_did, repo_did) values (?, ?, ?, ?);`,
14
+
`insert into collaborators (did, rkey, subject_did, repo_did) values (?, ?, ?, ?)
15
+
on conflict(repo_did, subject_did) do update set
16
+
did = excluded.did,
17
+
rkey = excluded.rkey`,
15
18
c.Did, c.Rkey, c.SubjectDid, string(c.RepoDid),
16
19
)
17
20
return err
+95
appview/db/collaborators_null_rkey_test.go
+95
appview/db/collaborators_null_rkey_test.go
···
1
+
package db
2
+
3
+
import (
4
+
"database/sql"
5
+
"testing"
6
+
7
+
"github.com/bluesky-social/indigo/atproto/syntax"
8
+
"tangled.org/core/appview/models"
9
+
"tangled.org/core/orm"
10
+
)
11
+
12
+
func TestGetCollaborators_NullRkey(t *testing.T) {
13
+
d := newTestDB(t)
14
+
15
+
seedRepo(t, d, "did:plc:boltless", "knot.example", "anemone", "anemone", "did:plc:anemone")
16
+
17
+
_, err := d.Exec(
18
+
`insert into collaborators (did, rkey, subject_did, repo_did) values (?, NULL, ?, ?)`,
19
+
"did:plc:boltless", "did:plc:akshay", "did:plc:anemone",
20
+
)
21
+
if err != nil {
22
+
t.Fatalf("insert: %v", err)
23
+
}
24
+
25
+
collabs, err := GetCollaborators(d, orm.FilterEq("repo_did", "did:plc:anemone"))
26
+
if err != nil {
27
+
t.Fatalf("GetCollaborators: %v", err)
28
+
}
29
+
if len(collabs) != 1 {
30
+
t.Fatalf("want 1 collab, got %d", len(collabs))
31
+
}
32
+
if collabs[0].Rkey.Valid {
33
+
t.Fatalf("expected NULL rkey to scan as !Valid, got %+v", collabs[0].Rkey)
34
+
}
35
+
}
36
+
37
+
func TestAddCollaborator_DuplicateUpdatesRkey(t *testing.T) {
38
+
d := newTestDB(t)
39
+
40
+
seedRepo(t, d, "did:plc:boltless", "knot.example", "anemone", "anemone", "did:plc:anemone")
41
+
42
+
collab := func(rkey string) models.Collaborator {
43
+
return models.Collaborator{
44
+
Did: syntax.DID("did:plc:boltless"),
45
+
Rkey: sql.NullString{String: rkey, Valid: true},
46
+
SubjectDid: syntax.DID("did:plc:akshay"),
47
+
RepoDid: syntax.DID("did:plc:anemone"),
48
+
}
49
+
}
50
+
51
+
if err := AddCollaborator(d, collab("rkey-first")); err != nil {
52
+
t.Fatalf("first AddCollaborator: %v", err)
53
+
}
54
+
if err := AddCollaborator(d, collab("rkey-second")); err != nil {
55
+
t.Fatalf("second AddCollaborator: %v", err)
56
+
}
57
+
58
+
collabs, err := GetCollaborators(d, orm.FilterEq("repo_did", "did:plc:anemone"))
59
+
if err != nil {
60
+
t.Fatalf("GetCollaborators: %v", err)
61
+
}
62
+
if len(collabs) != 1 {
63
+
t.Fatalf("want 1 collab after dup add, got %d", len(collabs))
64
+
}
65
+
if collabs[0].Rkey.String != "rkey-second" {
66
+
t.Fatalf("want rkey-second, got %q", collabs[0].Rkey.String)
67
+
}
68
+
}
69
+
70
+
func TestEnqueuePdsRewritesForRepo_SkipsNullRkeyCollab(t *testing.T) {
71
+
d := newTestDB(t)
72
+
73
+
seedRepo(t, d, "did:plc:boltless", "knot.example", "anemone", "anemone", "did:plc:anemone")
74
+
75
+
if _, err := d.Exec(
76
+
`insert into collaborators (did, rkey, subject_did, repo_did) values (?, NULL, ?, ?)`,
77
+
"did:plc:boltless", "did:plc:akshay", "did:plc:anemone",
78
+
); err != nil {
79
+
t.Fatalf("insert: %v", err)
80
+
}
81
+
82
+
tx, err := d.Begin()
83
+
if err != nil {
84
+
t.Fatalf("Begin: %v", err)
85
+
}
86
+
defer tx.Rollback()
87
+
88
+
if err := EnqueuePdsRewritesForRepo(tx, "did:plc:anemone", "at://did:plc:boltless/sh.tangled.repo/anemone"); err != nil {
89
+
t.Fatalf("EnqueuePdsRewritesForRepo: %v", err)
90
+
}
91
+
92
+
if err := tx.Commit(); err != nil {
93
+
t.Fatalf("Commit: %v", err)
94
+
}
95
+
}
+31
appview/db/db.go
+31
appview/db/db.go
···
1961
1961
return err
1962
1962
})
1963
1963
1964
+
orm.RunMigration(conn, logger, "collaborators-unique-on-repo-subject", func(tx *sql.Tx) error {
1965
+
_, err := tx.Exec(`
1966
+
CREATE TABLE collaborators_new (
1967
+
id INTEGER PRIMARY KEY AUTOINCREMENT,
1968
+
did TEXT NOT NULL,
1969
+
rkey TEXT,
1970
+
subject_did TEXT NOT NULL,
1971
+
repo_did TEXT NOT NULL,
1972
+
created TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')),
1973
+
UNIQUE(repo_did, subject_did),
1974
+
FOREIGN KEY (repo_did) REFERENCES repos(repo_did) ON DELETE CASCADE
1975
+
);
1976
+
INSERT INTO collaborators_new (id, did, rkey, subject_did, repo_did, created)
1977
+
SELECT id, did, rkey, subject_did, repo_did, created
1978
+
FROM (
1979
+
SELECT
1980
+
id, did, rkey, subject_did, repo_did, created,
1981
+
ROW_NUMBER() OVER (
1982
+
PARTITION BY repo_did, subject_did
1983
+
ORDER BY created DESC, id DESC
1984
+
) AS rn
1985
+
FROM collaborators
1986
+
)
1987
+
WHERE rn = 1;
1988
+
DROP TABLE collaborators;
1989
+
ALTER TABLE collaborators_new RENAME TO collaborators;
1990
+
CREATE INDEX idx_collaborators_repo_did ON collaborators(repo_did);
1991
+
CREATE INDEX idx_collaborators_subject_did ON collaborators(subject_did);
1992
+
`)
1993
+
return err
1994
+
})
1964
1995
return &DB{
1965
1996
db,
1966
1997
logger,
+6
-2
appview/db/repos.go
+6
-2
appview/db/repos.go
···
720
720
721
721
var pairs []struct{ did, rkey string }
722
722
for rows.Next() {
723
-
var d, r string
723
+
var d string
724
+
var r sql.NullString
724
725
if scanErr := rows.Scan(&d, &r); scanErr != nil {
725
726
rows.Close()
726
727
return fmt.Errorf("scan %s for pds rewrites: %w", src.table, scanErr)
727
728
}
728
-
pairs = append(pairs, struct{ did, rkey string }{d, r})
729
+
if !r.Valid {
730
+
continue
731
+
}
732
+
pairs = append(pairs, struct{ did, rkey string }{d, r.String})
729
733
}
730
734
rows.Close()
731
735
if rowsErr := rows.Err(); rowsErr != nil {
+2
-1
appview/models/collaborator.go
+2
-1
appview/models/collaborator.go
+14
-1
appview/repo/repo.go
+14
-1
appview/repo/repo.go
···
745
745
l = l.With("collaborator", collaboratorIdent.Handle)
746
746
l = l.With("knot", f.Knot)
747
747
748
+
existing, err := db.GetCollaborators(rp.db,
749
+
orm.FilterEq("repo_did", f.RepoDid),
750
+
orm.FilterEq("subject_did", collaboratorIdent.DID.String()),
751
+
)
752
+
if err != nil {
753
+
fail("Failed to check existing collaborators.", err)
754
+
return
755
+
}
756
+
if len(existing) > 0 {
757
+
fail(fmt.Sprintf("%s is already a collaborator.", collaboratorIdent.Handle), nil)
758
+
return
759
+
}
760
+
748
761
// announce this relation into the firehose, store into owners' pds
749
762
client, err := rp.oauth.AuthorizedClient(r)
750
763
if err != nil {
···
803
816
804
817
err = db.AddCollaborator(tx, models.Collaborator{
805
818
Did: syntax.DID(currentUser.Did),
806
-
Rkey: rkey,
819
+
Rkey: sql.NullString{String: rkey, Valid: true},
807
820
SubjectDid: collaboratorIdent.DID,
808
821
RepoDid: syntax.DID(f.RepoDid),
809
822
Created: createdAt,
+172
appview/notify/db/db_test.go
+172
appview/notify/db/db_test.go
···
1
+
package db_test
2
+
3
+
import (
4
+
"context"
5
+
"path/filepath"
6
+
"testing"
7
+
8
+
"github.com/bluesky-social/indigo/atproto/syntax"
9
+
appviewdb "tangled.org/core/appview/db"
10
+
"tangled.org/core/appview/models"
11
+
notifydb "tangled.org/core/appview/notify/db"
12
+
)
13
+
14
+
func TestNewIssue_DeliversWithNullRkeyCollaborator(t *testing.T) {
15
+
d := setupNotifyTestDB(t)
16
+
17
+
ownerDid := "did:plc:boltless"
18
+
repoDid := "did:plc:anemone"
19
+
collabDid := "did:plc:limpet"
20
+
authorDid := "did:plc:akshay"
21
+
22
+
repo := seedNotifyRepo(t, d, ownerDid, repoDid)
23
+
insertNullRkeyCollaborator(t, d, ownerDid, collabDid, repoDid)
24
+
25
+
issue := seedIssue(t, d, authorDid, repoDid, repo)
26
+
27
+
notifier := notifydb.NewDatabaseNotifier(d, nil)
28
+
notifier.NewIssue(context.Background(), issue, nil)
29
+
30
+
if got := notificationCount(t, d, ownerDid); got != 1 {
31
+
t.Errorf("repo owner %s: want 1 notification, got %d", ownerDid, got)
32
+
}
33
+
if got := notificationCount(t, d, collabDid); got != 1 {
34
+
t.Errorf("null-rkey collaborator %s: want 1 notification, got %d", collabDid, got)
35
+
}
36
+
if got := notificationCount(t, d, authorDid); got != 0 {
37
+
t.Errorf("issue author %s: want 0 notifications, got %d", authorDid, got)
38
+
}
39
+
}
40
+
41
+
func TestNewPull_DeliversWithNullRkeyCollaborator(t *testing.T) {
42
+
d := setupNotifyTestDB(t)
43
+
44
+
ownerDid := "did:plc:boltless"
45
+
repoDid := "did:plc:anemone"
46
+
collabDid := "did:plc:limpet"
47
+
authorDid := "did:plc:akshay"
48
+
49
+
seedNotifyRepo(t, d, ownerDid, repoDid)
50
+
insertNullRkeyCollaborator(t, d, ownerDid, collabDid, repoDid)
51
+
52
+
pull := seedPull(t, d, authorDid, repoDid)
53
+
54
+
notifier := notifydb.NewDatabaseNotifier(d, nil)
55
+
notifier.NewPull(context.Background(), pull)
56
+
57
+
if got := notificationCount(t, d, ownerDid); got != 1 {
58
+
t.Errorf("repo owner %s: want 1 notification, got %d", ownerDid, got)
59
+
}
60
+
if got := notificationCount(t, d, collabDid); got != 1 {
61
+
t.Errorf("null-rkey collaborator %s: want 1 notification, got %d", collabDid, got)
62
+
}
63
+
if got := notificationCount(t, d, authorDid); got != 0 {
64
+
t.Errorf("pull author %s: want 0 notifications, got %d", authorDid, got)
65
+
}
66
+
}
67
+
68
+
func setupNotifyTestDB(t *testing.T) *appviewdb.DB {
69
+
t.Helper()
70
+
path := filepath.Join(t.TempDir(), "test.db")
71
+
d, err := appviewdb.Make(context.Background(), path)
72
+
if err != nil {
73
+
t.Fatalf("Make: %v", err)
74
+
}
75
+
t.Cleanup(func() { d.Close() })
76
+
return d
77
+
}
78
+
79
+
func seedNotifyRepo(t *testing.T, d *appviewdb.DB, ownerDid, repoDid string) *models.Repo {
80
+
t.Helper()
81
+
tx, err := d.Begin()
82
+
if err != nil {
83
+
t.Fatalf("Begin: %v", err)
84
+
}
85
+
repo := &models.Repo{
86
+
Did: ownerDid,
87
+
Name: "shell",
88
+
Knot: "knot.example",
89
+
Rkey: "shellrkey",
90
+
RepoDid: repoDid,
91
+
}
92
+
if err := appviewdb.AddRepo(tx, repo); err != nil {
93
+
t.Fatalf("AddRepo: %v", err)
94
+
}
95
+
if err := tx.Commit(); err != nil {
96
+
t.Fatalf("Commit: %v", err)
97
+
}
98
+
return repo
99
+
}
100
+
101
+
func seedIssue(t *testing.T, d *appviewdb.DB, authorDid, repoDid string, repo *models.Repo) *models.Issue {
102
+
t.Helper()
103
+
issue := &models.Issue{
104
+
Did: authorDid,
105
+
Rkey: "issuerkey",
106
+
RepoDid: syntax.DID(repoDid),
107
+
IssueId: 1,
108
+
Title: "test",
109
+
Body: "body",
110
+
Open: true,
111
+
Repo: repo,
112
+
}
113
+
result, err := d.Exec(
114
+
`insert into issues (did, rkey, repo_did, issue_id, title, body, open) values (?, ?, ?, ?, ?, ?, 1)`,
115
+
issue.Did, issue.Rkey, string(issue.RepoDid), issue.IssueId, issue.Title, issue.Body,
116
+
)
117
+
if err != nil {
118
+
t.Fatalf("insert issue: %v", err)
119
+
}
120
+
id, err := result.LastInsertId()
121
+
if err != nil {
122
+
t.Fatalf("LastInsertId: %v", err)
123
+
}
124
+
issue.Id = id
125
+
return issue
126
+
}
127
+
128
+
func seedPull(t *testing.T, d *appviewdb.DB, authorDid, repoDid string) *models.Pull {
129
+
t.Helper()
130
+
pull := &models.Pull{
131
+
RepoDid: syntax.DID(repoDid),
132
+
OwnerDid: authorDid,
133
+
Rkey: "pullrkey",
134
+
Title: "test",
135
+
Body: "body",
136
+
TargetBranch: "main",
137
+
State: models.PullOpen,
138
+
}
139
+
tx, err := d.Begin()
140
+
if err != nil {
141
+
t.Fatalf("Begin: %v", err)
142
+
}
143
+
if err := appviewdb.PutPull(tx, pull); err != nil {
144
+
t.Fatalf("PutPull: %v", err)
145
+
}
146
+
if err := tx.Commit(); err != nil {
147
+
t.Fatalf("Commit: %v", err)
148
+
}
149
+
return pull
150
+
}
151
+
152
+
func insertNullRkeyCollaborator(t *testing.T, d *appviewdb.DB, issuerDid, subjectDid, repoDid string) {
153
+
t.Helper()
154
+
if _, err := d.Exec(
155
+
`insert into collaborators (did, rkey, subject_did, repo_did) values (?, NULL, ?, ?)`,
156
+
issuerDid, subjectDid, repoDid,
157
+
); err != nil {
158
+
t.Fatalf("insert null-rkey collaborator: %v", err)
159
+
}
160
+
}
161
+
162
+
func notificationCount(t *testing.T, d *appviewdb.DB, recipientDid string) int {
163
+
t.Helper()
164
+
var count int
165
+
if err := d.QueryRow(
166
+
`select count(*) from notifications where recipient_did = ?`,
167
+
recipientDid,
168
+
).Scan(&count); err != nil {
169
+
t.Fatalf("count notifications for %s: %v", recipientDid, err)
170
+
}
171
+
return count
172
+
}