Skip to content

Commit 01837b6

Browse files
paskalumputun
authored andcommitted
fix readonly status, deleted count for plain /find request
1 parent d020998 commit 01837b6

File tree

3 files changed

+31
-10
lines changed

3 files changed

+31
-10
lines changed

backend/app/rest/api/admin_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ func TestAdmin_ReadOnlyNoComments(t *testing.T) {
530530
err = json.Unmarshal([]byte(res), &comments)
531531
assert.NoError(t, err)
532532
assert.Equal(t, 0, len(comments.Comments), "should have 0 comments")
533-
assert.False(t, comments.Info.ReadOnly, "different from the plain format, should be fixed")
533+
assert.True(t, comments.Info.ReadOnly)
534534
t.Logf("%+v", comments)
535535
}
536536

backend/app/rest/api/rest_public.go

+14
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ type pubStore interface {
5050

5151
// GET /find?site=siteID&url=post-url&format=[tree|plain]&sort=[+/-time|+/-score|+/-controversy]&view=[user|all]&since=unix_ts_msec
5252
// find comments for given post. Returns in tree or plain formats, sorted
53+
//
54+
// When `url` parameter is not set (e.g. request is for site-wide comments), does not return deleted comments.
5355
func (s *public) findCommentsCtrl(w http.ResponseWriter, r *http.Request) {
5456
locator := store.Locator{SiteID: r.URL.Query().Get("site"), URL: r.URL.Query().Get("url")}
5557
sort := r.URL.Query().Get("sort")
@@ -93,6 +95,18 @@ func (s *public) findCommentsCtrl(w http.ResponseWriter, r *http.Request) {
9395
if info, ee := s.dataService.Info(locator, s.readOnlyAge); ee == nil {
9496
withInfo.Info = info
9597
}
98+
if !since.IsZero() { // if since is set, number of comments can be different from total in the DB
99+
withInfo.Info.Count = 0
100+
for _, c := range comments {
101+
if !c.Deleted {
102+
withInfo.Info.Count++
103+
}
104+
}
105+
}
106+
// post might be readonly without any comments, Info call will fail then and ReadOnly flag should be checked separately
107+
if !withInfo.Info.ReadOnly && locator.URL != "" && s.dataService.IsReadOnly(locator) {
108+
withInfo.Info.ReadOnly = true
109+
}
96110
b, e = encodeJSONWithHTML(withInfo)
97111
}
98112
return b, e

backend/app/rest/api/rest_public_test.go

+16-9
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,10 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
629629
assert.NoError(t, err)
630630
srv.Cache.Flush(cache.FlusherRequest{})
631631

632+
commentLocator.URL = "readonly-test"
633+
// set post without comments to read-only
634+
assert.NoError(t, srv.DataService.SetReadOnly(commentLocator, true))
635+
632636
sinceTenSecondsAgo := strconv.FormatInt(time.Now().Add(-time.Second*10).UnixNano()/1000000, 10)
633637
sinceTS := make([]string, 9)
634638
formattedTS := make([]string, 9)
@@ -650,10 +654,10 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
650654
{"url=test-url&since=" + sinceTenSecondsAgo, fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
651655
{"since=" + sinceTS[0], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
652656
{"url=test-url&since=" + sinceTS[0], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
653-
{"since=" + sinceTS[1], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
654-
{"url=test-url&since=" + sinceTS[1], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
655-
{"since=" + sinceTS[4], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
656-
{"url=test-url&since=" + sinceTS[4], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
657+
{"since=" + sinceTS[1], fmt.Sprintf(`"info":{"count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
658+
{"url=test-url&since=" + sinceTS[1], fmt.Sprintf(`"info":{"url":"test-url","count":5,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
659+
{"since=" + sinceTS[4], fmt.Sprintf(`"info":{"count":3,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
660+
{"url=test-url&since=" + sinceTS[4], fmt.Sprintf(`"info":{"url":"test-url","count":2,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
657661
{"format=tree", `"info":{"url":"test-url","count":7`},
658662
{"format=tree&url=test-url", `"info":{"url":"test-url","count":6`},
659663
{"format=tree&sort=+time", `"info":{"url":"test-url","count":7`},
@@ -666,12 +670,15 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
666670
{"sort=+score&url=test-url", fmt.Sprintf(`"score":10,"vote":0,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[2])},
667671
{"sort=-score", fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[8])},
668672
{"sort=-score&url=test-url", fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[6])},
669-
{"sort=-time&since=" + sinceTS[4], fmt.Sprintf(`"score":-1,"vote":0,"controversy":2.924017738212866,"time":%q}],"info":{"count":7`, formattedTS[4])},
670-
{"sort=-score&since=" + sinceTS[3], fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[8])},
671-
{"sort=-score&url=test-url&since=" + sinceTS[3], fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[6])},
672-
{"sort=+controversy&url=test-url&since=" + sinceTS[5], fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[6])},
673+
{"sort=-time&since=" + sinceTS[4], fmt.Sprintf(`"score":-1,"vote":0,"controversy":2.924017738212866,"time":%q}],"info":{"count":3`, formattedTS[4])},
674+
{"sort=-score&since=" + sinceTS[3], fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":4`, formattedTS[8])},
675+
{"sort=-score&url=test-url&since=" + sinceTS[3], fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":3`, formattedTS[6])},
676+
{"sort=+controversy&url=test-url&since=" + sinceTS[5], fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":1`, formattedTS[6])},
673677
// three comments of which last one deleted and doesn't have controversy so returned last
674-
{"sort=-controversy&url=test-url&since=" + sinceTS[5], fmt.Sprintf(`"score":0,"vote":0,"time":%q,"delete":true}],"info":{"url":"test-url","count":6`, formattedTS[7])},
678+
{"sort=-controversy&url=test-url&since=" + sinceTS[5], fmt.Sprintf(`"score":0,"vote":0,"time":%q,"delete":true}],"info":{"url":"test-url","count":1`, formattedTS[7])},
679+
// test readonly status for the post without comments
680+
{"url=readonly-test", `"info":{"count":0,"read_only":true`},
681+
{"format=tree&url=readonly-test", `"info":{"count":0,"read_only":true`},
675682
}
676683

677684
for _, tc := range testCases {

0 commit comments

Comments
 (0)