Docs: Added Contributing.md file#36
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughThis pull request adds a contribution guide, implements client-side concurrency control across multiple frontend pages to prevent duplicate submissions, refactors HTML structure and Tailwind configuration formatting, and hardens the enrollment API endpoint to gracefully handle duplicate join attempts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a repository-specific contribution guide and also introduces a few functional changes to the join/enrollment flow and frontend form submission behavior.
Changes:
- Added
CONTRIBUTING.mdwith repo-specific setup, workflows, and contribution expectations. - Updated join/enrollment handling to be explicitly idempotent (“Already joined activity”) instead of relying on
INSERT OR IGNORE. - Refactored several HTML pages and added a small in-flight action lock to prevent duplicate submissions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
CONTRIBUTING.md |
Adds a contribution guide tailored to the project’s stack and workflows. |
src/worker.py |
Makes /api/join explicitly handle “already joined” before/after insert. |
public/teach.html |
Formatting cleanup + in-flight locking for host create flows. |
public/login.html |
Formatting cleanup + in-flight locking for login/register flows. |
public/course.html |
Formatting cleanup + in-flight locking for join flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| existing = await env.DB.prepare( | ||
| "SELECT id FROM enrollments WHERE activity_id=? AND user_id=?" | ||
| ).bind(act_id, user["id"]).first() | ||
| if existing: | ||
| return ok(None, "Already joined activity") | ||
|
|
||
| enr_id = new_id() | ||
| try: | ||
| await env.DB.prepare( | ||
| "INSERT OR IGNORE INTO enrollments (id,activity_id,user_id,role)" | ||
| "INSERT INTO enrollments (id,activity_id,user_id,role)" | ||
| " VALUES (?,?,?,?)" | ||
| ).bind(enr_id, act_id, user["id"], role).run() | ||
| except Exception as e: | ||
| if "UNIQUE" in str(e): | ||
| return ok(None, "Already joined activity") | ||
| capture_exception(e, req, env, "api_join.insert_enrollment") | ||
| return err("Failed to join activity — please try again", 500) |
There was a problem hiding this comment.
The PR title/description indicate a docs-only change (adding CONTRIBUTING.md), but this PR also includes functional backend/frontend changes (e.g., join enrollment behavior and in-flight request locks). Please update the PR title and/or description to accurately reflect the full scope so reviewers can assess risk and release impact correctly.
| <div> | ||
| <label class="block text-sm font-medium text-slate-700 mb-1">Password</label> | ||
| <input id="r-password" type="password" autocomplete="new-password" required minlength="6" | ||
| class="w-full px-4 py-2.5 rounded-xl border border-slate-200 focus:outline-none focus:ring-2 focus:ring-brand/40 text-sm" | ||
| placeholder="min 6 characters" /> | ||
| </div> |
There was a problem hiding this comment.
The registration password field enforces minlength="6", but the backend rejects passwords shorter than 8 characters (see src/worker.py validation). This will cause avoidable registration failures; align the client-side minlength (and any helper text) with the server requirement.
| This project and everyone participating in it is expected to act respectfully and constructively. | ||
|
|
There was a problem hiding this comment.
Grammar: “everyone participating in it is expected” should be “everyone participating in it are expected” (or rephrase to avoid agreement issues, e.g., “everyone … is expected to act…”).
| const inFlight = new Set(); | ||
|
|
||
| async function withActionLock(key, action) { | ||
| if (inFlight.has(key)) return; | ||
| inFlight.add(key); | ||
| try { | ||
| await action(); | ||
| } finally { | ||
| inFlight.delete(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
The same withActionLock/inFlight helper is now duplicated inline across multiple HTML pages (login/course/teach). Consider extracting it to a shared script (or otherwise centralizing) to avoid future drift when the locking behavior needs to change.
Summary
Add a repository-specific contributing guide for learn and align contribution instructions with the actual project stack and workflows.
Changes
Added
CONTRIBUTING.mdtailored to the repo:Summary
This PR combines documentation improvements with bug fixes addressing duplicate submissions and improving idempotency:
Documentation Changes:
CONTRIBUTING.mdwith comprehensive contribution guidance including:.dev.varsconfiguration)Code Changes - Duplicate Submission Prevention:
Introduces client-side concurrency control across frontend pages and backend validation:
Frontend (
public/course.html,public/login.html,public/teach.html):inFlightSet andwithActionLock(key, action)helper to prevent concurrent submissions per actionBackend (
src/worker.py):api_jointo pre-check enrollment existence before database insertINSERT OR IGNOREto explicit pre-check + exception handlingImpact: