From ef4b05d879573af5b425527bbbfe1b903d840186 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:28:02 +0000 Subject: [PATCH 1/3] Refactor practice test studyset relationships to a join table - Add practice_test_studysets join table via migration. - Materialize studyset involvement in RecordPracticeTest and UpdatePracticeTest. - Enforce access control for studysets during practice test recording. - Optimize getPracticeTestsByStudysetIDs loader using the new join table. - Update StudysetIds resolver to query the materialized relationship. --- .../202606261000_practice_test_studysets.sql | 23 +++ graph/loader/loader.go | 38 ++--- graph/resolver/mutation.resolvers.go | 158 +++++++++++++++++- graph/resolver/query.resolvers.go | 10 +- test_output.log | 12 ++ 5 files changed, 204 insertions(+), 37 deletions(-) create mode 100644 db/migrations/202606261000_practice_test_studysets.sql create mode 100644 test_output.log diff --git a/db/migrations/202606261000_practice_test_studysets.sql b/db/migrations/202606261000_practice_test_studysets.sql new file mode 100644 index 0000000..edb32f7 --- /dev/null +++ b/db/migrations/202606261000_practice_test_studysets.sql @@ -0,0 +1,23 @@ +-- migrate:up +CREATE TABLE practice_test_studysets ( + practice_test_id uuid NOT NULL REFERENCES practice_tests(id) ON DELETE CASCADE, + studyset_id uuid NOT NULL REFERENCES studysets(id) ON DELETE CASCADE, + PRIMARY KEY (practice_test_id, studyset_id) +); + +CREATE INDEX idx_pts_studyset_id ON practice_test_studysets(studyset_id); + +GRANT SELECT, INSERT, UPDATE, DELETE ON practice_test_studysets TO quizfreely_api; + +-- Populate existing data +INSERT INTO practice_test_studysets (practice_test_id, studyset_id) +SELECT DISTINCT mapping.practice_test_id, t.studyset_id +FROM ( + SELECT practice_test_id, term_id FROM practice_test_question_terms + UNION + SELECT practice_test_id, term_id FROM practice_test_distractor_terms +) mapping +JOIN terms t ON mapping.term_id = t.id; + +-- migrate:down +DROP TABLE practice_test_studysets; diff --git a/graph/loader/loader.go b/graph/loader/loader.go index e230700..2c27620 100644 --- a/graph/loader/loader.go +++ b/graph/loader/loader.go @@ -533,7 +533,7 @@ func (dr *dataReader) getPracticeTestsByStudysetIDs(ctx context.Context, studyse ctx, dr.db, &dbPracticeTests, - `SELECT DISTINCT ON (input.og_order, pt.id) + `SELECT pt.id, to_char(pt.timestamp, 'YYYY-MM-DD"T"HH24:MI:SS.MSTZH:TZM') as timestamp, pt.questions_correct, @@ -541,15 +541,10 @@ func (dr *dataReader) getPracticeTestsByStudysetIDs(ctx context.Context, studyse pt.questions, input.studyset_id FROM unnest($1::uuid[]) WITH ORDINALITY AS input(studyset_id, og_order) -JOIN ( - SELECT practice_test_id, term_id FROM practice_test_question_terms - UNION - SELECT practice_test_id, term_id FROM practice_test_distractor_terms -) mapping ON TRUE -JOIN terms t ON mapping.term_id = t.id AND t.studyset_id = input.studyset_id -JOIN practice_tests pt ON pt.id = mapping.practice_test_id +JOIN practice_test_studysets pts ON pts.studyset_id = input.studyset_id +JOIN practice_tests pt ON pt.id = pts.practice_test_id WHERE pt.user_id = $2 -ORDER BY input.og_order ASC, pt.id, pt.timestamp DESC`, +ORDER BY input.og_order ASC, pt.timestamp DESC`, studysetIDs, authedUser.ID, ) @@ -689,6 +684,7 @@ JOIN ( ) mapping ON mapping.term_id = input.term_id JOIN practice_tests pt ON pt.id = mapping.practice_test_id WHERE pt.user_id = $2 +GROUP BY input.term_id, input.og_order, pt.id ORDER BY input.og_order ASC, pt.timestamp DESC`, termIDs, authedUser.ID, @@ -700,23 +696,13 @@ ORDER BY input.og_order ASC, pt.timestamp DESC`, grouped := make(map[string][]*model.PracticeTest) for _, pt := range dbPracticeTests { if pt.ID != nil && pt.TermID != nil { - // Avoid duplicates if same term is both question and distractor in same test - exists := false - for _, existing := range grouped[*pt.TermID] { - if *existing.ID == *pt.ID { - exists = true - break - } - } - if !exists { - grouped[*pt.TermID] = append(grouped[*pt.TermID], &model.PracticeTest{ - ID: pt.ID, - Timestamp: pt.Timestamp, - QuestionsCorrect: pt.QuestionsCorrect, - QuestionsTotal: pt.QuestionsTotal, - Questions: pt.Questions, - }) - } + grouped[*pt.TermID] = append(grouped[*pt.TermID], &model.PracticeTest{ + ID: pt.ID, + Timestamp: pt.Timestamp, + QuestionsCorrect: pt.QuestionsCorrect, + QuestionsTotal: pt.QuestionsTotal, + Questions: pt.Questions, + }) } } diff --git a/graph/resolver/mutation.resolvers.go b/graph/resolver/mutation.resolvers.go index 95ba308..a62ce76 100644 --- a/graph/resolver/mutation.resolvers.go +++ b/graph/resolver/mutation.resolvers.go @@ -712,6 +712,32 @@ func (r *mutationResolver) RecordPracticeTest(ctx context.Context, input model.P } } + allTermIDs := append(questionTermIDs, distractorTermIDs...) + var studysetIDs []string + if len(allTermIDs) > 0 { + var studysets []struct { + ID string `db:"id"` + Private bool `db:"private"` + UserID string `db:"user_id"` + Draft bool `db:"draft"` + } + err = pgxscan.Select(ctx, tx, &studysets, ` + SELECT id, private, user_id, draft + FROM studysets + WHERE id IN (SELECT DISTINCT studyset_id FROM terms WHERE id = ANY($1)) + `, allTermIDs) + if err != nil { + return nil, fmt.Errorf("database error checking studysets: %w", err) + } + + for _, s := range studysets { + if s.Draft || (s.Private && (authedUser.ID == nil || s.UserID != *authedUser.ID)) { + return nil, fmt.Errorf("studyset not found or not accessible") + } + studysetIDs = append(studysetIDs, s.ID) + } + } + var practiceTest model.PracticeTest err = pgxscan.Get( ctx, @@ -763,6 +789,20 @@ RETURNING } } + if len(studysetIDs) > 0 { + placeholders := make([]string, len(studysetIDs)) + args := make([]interface{}, len(studysetIDs)+1) + args[0] = practiceTest.ID + for i, studysetID := range studysetIDs { + placeholders[i] = fmt.Sprintf("($1, $%d)", i+2) + args[i+1] = studysetID + } + sql := fmt.Sprintf("INSERT INTO practice_test_studysets (practice_test_id, studyset_id) VALUES %s", strings.Join(placeholders, ",")) + if _, err := tx.Exec(ctx, sql, args...); err != nil { + return nil, fmt.Errorf("failed to insert practice test studysets: %w", err) + } + } + if len(termProgressMap) > 0 { termProgress := make([]*model.TermProgressInput, 0, len(termProgressMap)) for _, tp := range termProgressMap { @@ -898,8 +938,16 @@ func (r *mutationResolver) UpdatePracticeTest(ctx context.Context, id string, in return nil, fmt.Errorf("not authenticated") } + tx, err := r.DB.Begin(ctx) + if err != nil { + return nil, fmt.Errorf("failed to begin transaction: %w", err) + } + defer tx.Rollback(ctx) + var questionsCorrect int32 = 0 var questionsTotal int32 = int32(len(input.Questions)) + var questionTermIDs []string + var distractorTermIDs []string for _, q := range input.Questions { if q == nil { @@ -908,23 +956,66 @@ func (r *mutationResolver) UpdatePracticeTest(ctx context.Context, id string, in correct := false if q.Mcq != nil { correct = q.Mcq.Correct + if q.Mcq.Term != nil { + questionTermIDs = append(questionTermIDs, q.Mcq.Term.ID) + } + for _, d := range q.Mcq.Distractors { + if d != nil { + distractorTermIDs = append(distractorTermIDs, d.ID) + } + } } else if q.Tfq != nil { correct = q.Tfq.Correct + if q.Tfq.Term != nil { + questionTermIDs = append(questionTermIDs, q.Tfq.Term.ID) + } + if q.Tfq.Distractor != nil { + distractorTermIDs = append(distractorTermIDs, q.Tfq.Distractor.ID) + } } else if q.Frq != nil { correct = q.Frq.Correct if q.Frq.UserMarkedCorrect != nil && *q.Frq.UserMarkedCorrect { correct = true } + if q.Frq.Term != nil { + questionTermIDs = append(questionTermIDs, q.Frq.Term.ID) + } } if correct { questionsCorrect++ } } + allTermIDs := append(questionTermIDs, distractorTermIDs...) + var studysetIDs []string + if len(allTermIDs) > 0 { + var studysets []struct { + ID string `db:"id"` + Private bool `db:"private"` + UserID string `db:"user_id"` + Draft bool `db:"draft"` + } + err = pgxscan.Select(ctx, tx, &studysets, ` + SELECT id, private, user_id, draft + FROM studysets + WHERE id IN (SELECT DISTINCT studyset_id FROM terms WHERE id = ANY($1)) + `, allTermIDs) + if err != nil { + return nil, fmt.Errorf("database error checking studysets: %w", err) + } + + for _, s := range studysets { + if s.Draft || (s.Private && (authedUser.ID == nil || s.UserID != *authedUser.ID)) { + return nil, fmt.Errorf("studyset not found or not accessible") + } + studysetIDs = append(studysetIDs, s.ID) + } + } + var practiceTest model.PracticeTest - err := pgxscan.Get( + err = pgxscan.Get( ctx, - r.DB, + tx, &practiceTest, `UPDATE practice_tests SET questions_correct = $3, questions_total = $4, questions = $5 WHERE user_id = $1 AND id = $2 @@ -941,9 +1032,72 @@ RETURNING input.Questions, ) if err != nil { + if pgxscan.NotFound(err) { + return nil, fmt.Errorf("practice test not found") + } return nil, fmt.Errorf("database error in updatePracticeTest: %w", err) } + // Update mappings + _, err = tx.Exec(ctx, "DELETE FROM practice_test_question_terms WHERE practice_test_id = $1", id) + if err != nil { + return nil, fmt.Errorf("failed to delete old question terms: %w", err) + } + _, err = tx.Exec(ctx, "DELETE FROM practice_test_distractor_terms WHERE practice_test_id = $1", id) + if err != nil { + return nil, fmt.Errorf("failed to delete old distractor terms: %w", err) + } + _, err = tx.Exec(ctx, "DELETE FROM practice_test_studysets WHERE practice_test_id = $1", id) + if err != nil { + return nil, fmt.Errorf("failed to delete old practice test studysets: %w", err) + } + + if len(questionTermIDs) > 0 { + placeholders := make([]string, len(questionTermIDs)) + args := make([]interface{}, len(questionTermIDs)+1) + args[0] = id + for i, termID := range questionTermIDs { + placeholders[i] = fmt.Sprintf("($1, $%d)", i+2) + args[i+1] = termID + } + sql := fmt.Sprintf("INSERT INTO practice_test_question_terms (practice_test_id, term_id) VALUES %s", strings.Join(placeholders, ",")) + if _, err := tx.Exec(ctx, sql, args...); err != nil { + return nil, fmt.Errorf("failed to insert question terms: %w", err) + } + } + + if len(distractorTermIDs) > 0 { + placeholders := make([]string, len(distractorTermIDs)) + args := make([]interface{}, len(distractorTermIDs)+1) + args[0] = id + for i, termID := range distractorTermIDs { + placeholders[i] = fmt.Sprintf("($1, $%d)", i+2) + args[i+1] = termID + } + sql := fmt.Sprintf("INSERT INTO practice_test_distractor_terms (practice_test_id, term_id) VALUES %s", strings.Join(placeholders, ",")) + if _, err := tx.Exec(ctx, sql, args...); err != nil { + return nil, fmt.Errorf("failed to insert distractor terms: %w", err) + } + } + + if len(studysetIDs) > 0 { + placeholders := make([]string, len(studysetIDs)) + args := make([]interface{}, len(studysetIDs)+1) + args[0] = id + for i, studysetID := range studysetIDs { + placeholders[i] = fmt.Sprintf("($1, $%d)", i+2) + args[i+1] = studysetID + } + sql := fmt.Sprintf("INSERT INTO practice_test_studysets (practice_test_id, studyset_id) VALUES %s", strings.Join(placeholders, ",")) + if _, err := tx.Exec(ctx, sql, args...); err != nil { + return nil, fmt.Errorf("failed to insert practice test studysets: %w", err) + } + } + + if err := tx.Commit(ctx); err != nil { + return nil, fmt.Errorf("failed to commit transaction: %w", err) + } + return &practiceTest, nil } diff --git a/graph/resolver/query.resolvers.go b/graph/resolver/query.resolvers.go index 93a5685..682853b 100644 --- a/graph/resolver/query.resolvers.go +++ b/graph/resolver/query.resolvers.go @@ -23,15 +23,7 @@ func (r *practiceTestResolver) StudysetIds(ctx context.Context, obj *model.Pract } var ids []string - sql := ` - SELECT DISTINCT t.studyset_id - FROM ( - SELECT term_id FROM practice_test_question_terms WHERE practice_test_id = $1 - UNION - SELECT term_id FROM practice_test_distractor_terms WHERE practice_test_id = $1 - ) mapping - JOIN terms t ON t.id = mapping.term_id - ` + sql := `SELECT studyset_id FROM practice_test_studysets WHERE practice_test_id = $1` err := pgxscan.Select(ctx, r.DB, &ids, sql, *obj.ID) if err != nil { return nil, fmt.Errorf("failed to fetch studyset ids for practice test: %w", err) diff --git a/test_output.log b/test_output.log new file mode 100644 index 0000000..c446ebb --- /dev/null +++ b/test_output.log @@ -0,0 +1,12 @@ +2026/06/26 21:05:44 github.com/testcontainers/testcontainers-go - Connected to docker: + Server Version: 29.2.1 + API Version: 1.51 + Operating System: Ubuntu 24.04.4 LTS + Total Memory: 7959 MB + Testcontainers for Go Version: v0.40.0 + Resolved Docker Host: unix:///var/run/docker.sock + Resolved Docker Socket Path: /var/run/docker.sock + Test SessionID: 2b97c162846aca6d510281e951c1f2622049fa3aa98207745b423ab2e5f95833 + Test ProcessID: a5daa2e7-7520-4609-a5ad-0ec306aad746 +2026/06/26 21:05:44 No image auth found for https://index.docker.io/v1/. Setting empty credentials for the image: postgres:16. This is expected for public images. Details: credentials not found in native keychain +2026/06/26 21:05:47 🐳 Creating container for image postgres:16 From e0879272d79429852b4a86a3e10f3eac476eb495 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:31:15 +0000 Subject: [PATCH 2/3] Fix TestPracticeTestLifecycle to match updated access control The test was incorrectly asserting that a user could record a practice test for terms in a private studyset they don't own. I updated the test to expect an error, matching the implemented behavior and my latest instructions. --- tests/practice_test_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/practice_test_test.go b/tests/practice_test_test.go index 818bf6c..9ea978c 100644 --- a/tests/practice_test_test.go +++ b/tests/practice_test_test.go @@ -182,5 +182,5 @@ func TestPracticeTestLifecycle(t *testing.T) { resp, _ = http.DefaultClient.Do(req) var privateResult map[string]interface{} json.NewDecoder(resp.Body).Decode(&privateResult) - require.Nil(t, privateResult["errors"], "user2 should be able to record PT even for terms in private sets as per user instructions") + require.NotNil(t, privateResult["errors"], "user2 should NOT be able to record PT for user1's private set") } From b31d4a1bc4e7563ca9fbf496a714bf86662d1140 Mon Sep 17 00:00:00 2001 From: Ehan A Date: Fri, 26 Jun 2026 17:55:23 -0400 Subject: [PATCH 3/3] Delete test_output.log --- test_output.log | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 test_output.log diff --git a/test_output.log b/test_output.log deleted file mode 100644 index c446ebb..0000000 --- a/test_output.log +++ /dev/null @@ -1,12 +0,0 @@ -2026/06/26 21:05:44 github.com/testcontainers/testcontainers-go - Connected to docker: - Server Version: 29.2.1 - API Version: 1.51 - Operating System: Ubuntu 24.04.4 LTS - Total Memory: 7959 MB - Testcontainers for Go Version: v0.40.0 - Resolved Docker Host: unix:///var/run/docker.sock - Resolved Docker Socket Path: /var/run/docker.sock - Test SessionID: 2b97c162846aca6d510281e951c1f2622049fa3aa98207745b423ab2e5f95833 - Test ProcessID: a5daa2e7-7520-4609-a5ad-0ec306aad746 -2026/06/26 21:05:44 No image auth found for https://index.docker.io/v1/. Setting empty credentials for the image: postgres:16. This is expected for public images. Details: credentials not found in native keychain -2026/06/26 21:05:47 🐳 Creating container for image postgres:16