Skip to content

feat: Disable sign up for xandylearning subdomain - #547

Closed
HarishV14 wants to merge 3 commits into
masterfrom
feat/disable-signup-xandylearning
Closed

feat: Disable sign up for xandylearning subdomain#547
HarishV14 wants to merge 3 commits into
masterfrom
feat/disable-signup-xandylearning

Conversation

@HarishV14

Copy link
Copy Markdown
Contributor
  • Added "xandylearning" to the list of disallowed subdomains for sign up in Constants
  • Hide the sign up button in LoginActivity and UsernameAuthentication if
    the current subdomain matches the disallowed list

Todo -
https://app.basecamp.com/4160028/buckets/48341145/card_tables/cards/10235987664

@claude

claude Bot commented Aug 26, 2026

Copy link
Copy Markdown

Claude finished @HarishV14's task in 1m 28s —— View job


Code Review

Todo list

  • Read trigger comment and gather PR context
  • Read changed files (Constants.java, LoginActivity.java, UsernameAuthentication.kt)
  • Check CLAUDE.md for repo-specific guidelines (none found in repo)
  • Review code quality, security, performance, testing, docs, Android/Kotlin rules
  • Post final review feedback

Summary

Small, focused change that hides the sign-up button for the xandylearning subdomain by checking a new Constants.DISALLOWED_SIGNUP_SUBDOMAINS list against R.string.testpress_site_subdomain (a build-time flavor value set from config.json, same pattern already used in AppChecker.kt). As a side effect it also fixes a pre-existing potential NullPointerException in LoginActivity.java where instituteSettings.getAllowSignup() (a boxed Boolean) was being auto-unboxed without a null check — good catch. 👍

Observations

  1. Duplicated logic across two files (app/src/main/java/in/testpress/testpress/authenticator/LoginActivity.java:456 and app/src/main/java/in/testpress/testpress/ui/fragments/UsernameAuthentication.kt:191) — the "is signup disallowed for this subdomain" check is implemented independently in Java (Arrays.asList(...).contains(...)) and Kotlin (Array.contains(...)). Since AppChecker.kt already centralizes similar per-subdomain checks (isEduportApp, isLmsDemoApp, isCatkingApp), consider adding a AppChecker.isSignupDisabled(context) (or a helper on Constants) and calling it from both places. This avoids the two checks drifting if the disallowed list or matching logic changes later.
    Fix this →

  2. UI-only enforcement: this change only hides the sign-up button/view; it doesn't appear to block the underlying registration flow at the network/API layer (Constants.Http.URL_REGISTER_FRAG). That matches the PR's stated scope ("Hide the sign up button"), but worth confirming with the backend/Basecamp card that server-side signup is also disabled for xandylearning, so a user can't bypass the hidden button (e.g. via a modified/older client build).

  3. Minor/nit: Arrays.asList(Constants.DISALLOWED_SIGNUP_SUBDOMAINS).contains(...) in LoginActivity.java:456 allocates a new List on every call to updateInstituteSpecificFields(). Negligible given the array has one element today, but if the list grows, a Set<String> (or the shared helper suggested above) would be both cleaner and avoid repeated allocation.

  4. No new tests were added, but this is consistent with the existing test coverage in this area (no prior tests exist for LoginActivity/UsernameAuthentication sign-up visibility), so not a blocker.

  5. Hardcoding "xandylearning" as a raw string in Constants.java is fine here — it's a build/subdomain identifier, not user-facing text, and matches the existing convention in AppChecker.kt.

Overall this is a safe, low-risk change that achieves its stated goal. The main suggestion is consolidating the duplicated subdomain check (#1) for maintainability.

@HarishV14 HarishV14 closed this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant