feat: add signup pause flag#249
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces signup gating for TuxSEO via a new ChangesSignup Gating Feature
Sequence DiagramsequenceDiagram
participant User
participant SignupFlow
participant Adapter
participant Settings
participant ClosedTemplate
User->>SignupFlow: Attempt to register
SignupFlow->>Adapter: is_open_for_signup(request)?
Adapter->>Settings: Check ALLOW_SIGNUPS value
Settings-->>Adapter: False
Adapter-->>SignupFlow: Signup blocked
SignupFlow->>ClosedTemplate: Render signup_closed.html
ClosedTemplate-->>User: Display "Signups paused" + login link
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile SummaryThis PR introduces an
Confidence Score: 4/5Safe to merge; the flag defaults to open, so existing behaviour is unchanged until an operator explicitly sets ALLOW_SIGNUPS=false. Both adapter overrides return settings.ALLOW_SIGNUPS directly without chaining to super(). If allauth ever adds conditions of its own to is_open_for_signup, those checks would be silently bypassed when ALLOW_SIGNUPS=True. The closed path works correctly and the template is well-formed; the risk is confined to edge cases in the open path. core/adapters.py — both is_open_for_signup methods skip super(). Important Files Changed
Reviews (1): Last reviewed commit: "feat: add signup pause flag" | Re-trigger Greptile |
| def is_open_for_signup(self, request): | ||
| """Allow operators to pause new registrations without affecting existing users.""" | ||
| return settings.ALLOW_SIGNUPS |
There was a problem hiding this comment.
Both
is_open_for_signup overrides return settings.ALLOW_SIGNUPS directly without calling super(). If a future version of allauth (or an intermediate custom adapter) adds additional conditions to is_open_for_signup — such as a max-user cap, IP-based blocks, or another admin flag — those checks would be silently bypassed. Chaining to super() is the safe pattern here and costs nothing when ALLOW_SIGNUPS is True.
| def is_open_for_signup(self, request): | |
| """Allow operators to pause new registrations without affecting existing users.""" | |
| return settings.ALLOW_SIGNUPS | |
| def is_open_for_signup(self, request): | |
| """Allow operators to pause new registrations without affecting existing users.""" | |
| return settings.ALLOW_SIGNUPS and super().is_open_for_signup(request) |
| def is_open_for_signup(self, request, sociallogin): | ||
| """Mirror email signup gating for social-account auto-signups.""" | ||
| return settings.ALLOW_SIGNUPS |
There was a problem hiding this comment.
Same super()-bypass concern as
CustomAccountAdapter.is_open_for_signup. Chaining ensures any allauth-level or future adapter-level guard is still honoured.
| def is_open_for_signup(self, request, sociallogin): | |
| """Mirror email signup gating for social-account auto-signups.""" | |
| return settings.ALLOW_SIGNUPS | |
| def is_open_for_signup(self, request, sociallogin): | |
| """Mirror email signup gating for social-account auto-signups.""" | |
| return settings.ALLOW_SIGNUPS and super().is_open_for_signup(request, sociallogin) |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/templates/account/signup_closed.html (1)
4-18: ⚡ Quick winUse semantic page landmarks for the closed-signup view.
Line 4 and Line 5 currently use generic container
divs; switching tomain/sectionimproves document semantics and accessibility with minimal change.♻️ Proposed semantic structure update
-<div class="flex justify-center items-center px-4 py-16 my-4 sm:px-6 lg:px-8"> - <div class="space-y-6 w-full max-w-md text-center"> - <div> +<main class="flex justify-center items-center px-4 py-16 my-4 sm:px-6 lg:px-8"> + <section class="space-y-6 w-full max-w-md text-center" aria-labelledby="signup-closed-title"> + <div> <p class="text-sm font-semibold tracking-wide text-gray-500 uppercase">Signups paused</p> - <h1 class="mt-3 text-3xl font-extrabold text-gray-900">TuxSEO is not accepting new accounts right now.</h1> + <h1 id="signup-closed-title" class="mt-3 text-3xl font-extrabold text-gray-900">TuxSEO is not accepting new accounts right now.</h1> <p class="mt-4 text-sm leading-6 text-gray-600"> Existing users can continue using the product as usual. New account creation is paused for now. </p> </div> <a href="{% url 'account_login' %}" class="inline-flex justify-center px-5 py-3 text-sm font-semibold text-white bg-gray-800 rounded-md hover:bg-gray-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-gray-500"> Log in to your account </a> - </div> -</div> + </section> +</main>As per coding guidelines, "frontend/templates/**/*.html: Use semantic HTML elements (dialog, details/summary, etc.) in Django templates."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/templates/account/signup_closed.html` around lines 4 - 18, Replace the two outer generic div containers with semantic landmarks: change the outermost div (class="flex justify-center items-center px-4 py-16 my-4 sm:px-6 lg:px-8") to a <main> element, and change the inner div (class="space-y-6 w-full max-w-md text-center") to a <section> with a descriptive accessible name (e.g., add role="region" and aria-labelledby pointing to the heading); add an id to the existing <h1> (e.g., id="signup-closed-heading") and reference it from the section's aria-labelledby, and keep all existing classes and the login anchor ({% url 'account_login' %}) unchanged so layout and links remain identical while improving semantics and accessibility.core/tests/test_signup_gating.py (1)
29-40: ⚡ Quick winPrefer rendered-template assertions over raw-file string checks.
The test reads template source directly via
read_text(), so template rendering failures (e.g., broken{% url %}tags or syntax errors) would not be caught. Userender_to_string()to validate that the template compiles and renders correctly. Note: The template appears to be unused—no view currently renderssignup_closed.htmlwhen signups close; consider whether this template is reachable or if it should be wired into the signup flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/tests/test_signup_gating.py` around lines 29 - 40, The test test_signup_closed_template_tells_existing_users_to_log_in reads the template file source directly which won’t catch template rendering/compilation errors; change it to render the template using Django's render_to_string (import render_to_string) for "account/signup_closed.html" and run assertions against the rendered string (keep the same assertions for "Signups paused", "Existing users can continue using the product as usual", "account_login", and absence of 'name="email"' and 'name="password1"'); also note the template may be unused—add a brief check or TODO to ensure signup_closed.html is actually wired into the signup flow or a view renders it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/tests/test_signup_gating.py`:
- Around line 29-40: The test
test_signup_closed_template_tells_existing_users_to_log_in reads the template
file source directly which won’t catch template rendering/compilation errors;
change it to render the template using Django's render_to_string (import
render_to_string) for "account/signup_closed.html" and run assertions against
the rendered string (keep the same assertions for "Signups paused", "Existing
users can continue using the product as usual", "account_login", and absence of
'name="email"' and 'name="password1"'); also note the template may be unused—add
a brief check or TODO to ensure signup_closed.html is actually wired into the
signup flow or a view renders it.
In `@frontend/templates/account/signup_closed.html`:
- Around line 4-18: Replace the two outer generic div containers with semantic
landmarks: change the outermost div (class="flex justify-center items-center
px-4 py-16 my-4 sm:px-6 lg:px-8") to a <main> element, and change the inner div
(class="space-y-6 w-full max-w-md text-center") to a <section> with a
descriptive accessible name (e.g., add role="region" and aria-labelledby
pointing to the heading); add an id to the existing <h1> (e.g.,
id="signup-closed-heading") and reference it from the section's aria-labelledby,
and keep all existing classes and the login anchor ({% url 'account_login' %})
unchanged so layout and links remain identical while improving semantics and
accessibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a621a84-9a7e-4c10-87f0-68e9b487c59f
📒 Files selected for processing (5)
CHANGELOG.mdcore/adapters.pycore/tests/test_signup_gating.pyfrontend/templates/account/signup_closed.htmltuxseo/settings.py
Summary
ALLOW_SIGNUPSenv flag (defaults totrue) for pausing new registrationsVerification
uv run pytest core/tests/test_signup_gating.py core/tests/test_forms.py::TestCustomSignUpFormTurnstile -quv run --with ruff ruff check tuxseo/settings.py core/adapters.py core/tests/test_signup_gating.pyuv run --with ruff ruff format --check tuxseo/settings.py core/adapters.py core/tests/test_signup_gating.pyNote: full DB-backed signup page test could not run in this container because the configured Postgres host
dbis not resolvable here.Summary by CodeRabbit
ALLOW_SIGNUPSconfiguration option to pause new user registrations while keeping existing users able to log in. A signups-closed page guides users accordingly.