Skip to content

Commit

Permalink
fix readonly status, deleted count for plain /find request
Browse files Browse the repository at this point in the history
  • Loading branch information
@paskal @umputun
paskal authored and umputun committed Mar 4, 2024
1 parent d020998 commit 01837b6
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
2 changes: 1 addition & 1 deletion backend/app/rest/api/admin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func TestAdmin_ReadOnlyNoComments(t *testing.T) {
err = json.Unmarshal([]byte(res), &comments)
assert.NoError(t, err)
assert.Equal(t, 0, len(comments.Comments), "should have 0 comments")
assert.False(t, comments.Info.ReadOnly, "different from the plain format, should be fixed")
assert.True(t, comments.Info.ReadOnly)
t.Logf("%+v", comments)
}

Expand Down
14 changes: 14 additions & 0 deletions backend/app/rest/api/rest_public.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ type pubStore interface {

// GET /find?site=siteID&url=post-url&format=[tree|plain]&sort=[+/-time|+/-score|+/-controversy]&view=[user|all]&since=unix_ts_msec
// find comments for given post. Returns in tree or plain formats, sorted
//
// When `url` parameter is not set (e.g. request is for site-wide comments), does not return deleted comments.
func (s *public) findCommentsCtrl(w http.ResponseWriter, r *http.Request) {
locator := store.Locator{SiteID: r.URL.Query().Get("site"), URL: r.URL.Query().Get("url")}
sort := r.URL.Query().Get("sort")
Expand Down Expand Up @@ -93,6 +95,18 @@ func (s *public) findCommentsCtrl(w http.ResponseWriter, r *http.Request) {
if info, ee := s.dataService.Info(locator, s.readOnlyAge); ee == nil {
withInfo.Info = info
}
if !since.IsZero() { // if since is set, number of comments can be different from total in the DB
withInfo.Info.Count = 0
for _, c := range comments {
if !c.Deleted {
withInfo.Info.Count++
}
}
}
// post might be readonly without any comments, Info call will fail then and ReadOnly flag should be checked separately
if !withInfo.Info.ReadOnly && locator.URL != "" && s.dataService.IsReadOnly(locator) {
withInfo.Info.ReadOnly = true
}
b, e = encodeJSONWithHTML(withInfo)
}
return b, e
Expand Down
25 changes: 16 additions & 9 deletions backend/app/rest/api/rest_public_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,10 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
assert.NoError(t, err)
srv.Cache.Flush(cache.FlusherRequest{})

commentLocator.URL = "readonly-test"
// set post without comments to read-only
assert.NoError(t, srv.DataService.SetReadOnly(commentLocator, true))

sinceTenSecondsAgo := strconv.FormatInt(time.Now().Add(-time.Second*10).UnixNano()/1000000, 10)
sinceTS := make([]string, 9)
formattedTS := make([]string, 9)
Expand All @@ -650,10 +654,10 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
{"url=test-url&since=" + sinceTenSecondsAgo, fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[0], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[0], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[1], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[1], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[4], fmt.Sprintf(`"info":{"count":7,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[4], fmt.Sprintf(`"info":{"url":"test-url","count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[1], fmt.Sprintf(`"info":{"count":6,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[1], fmt.Sprintf(`"info":{"url":"test-url","count":5,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[4], fmt.Sprintf(`"info":{"count":3,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[4], fmt.Sprintf(`"info":{"url":"test-url","count":2,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"format=tree", `"info":{"url":"test-url","count":7`},
{"format=tree&url=test-url", `"info":{"url":"test-url","count":6`},
{"format=tree&sort=+time", `"info":{"url":"test-url","count":7`},
Expand All @@ -666,12 +670,15 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
{"sort=+score&url=test-url", fmt.Sprintf(`"score":10,"vote":0,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[2])},
{"sort=-score", fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[8])},
{"sort=-score&url=test-url", fmt.Sprintf(`"score":-2,"vote":0,"controversy":1.5874010519681994,"time":%q}],"info":{"url":"test-url","count":6`, formattedTS[6])},
{"sort=-time&since=" + sinceTS[4], fmt.Sprintf(`"score":-1,"vote":0,"controversy":2.924017738212866,"time":%q}],"info":{"count":7`, formattedTS[4])},
{"sort=-score&since=" + sinceTS[3], fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[8])},
{"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])},
{"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])},
{"sort=-time&since=" + sinceTS[4], fmt.Sprintf(`"score":-1,"vote":0,"controversy":2.924017738212866,"time":%q}],"info":{"count":3`, formattedTS[4])},
{"sort=-score&since=" + sinceTS[3], fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":4`, formattedTS[8])},
{"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])},
{"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])},
// three comments of which last one deleted and doesn't have controversy so returned last
{"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])},
{"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])},
// test readonly status for the post without comments
{"url=readonly-test", `"info":{"count":0,"read_only":true`},
{"format=tree&url=readonly-test", `"info":{"count":0,"read_only":true`},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 01837b6

Please sign in to comment.