Skip to content

Conversation

@alexdevz
Copy link
Member

Added eligibility note for US-based organizations.

Summary of the problem

Costs for intl shipping are too high- decision has been made to be US-only for the time being until we can find a solution.

Describe your changes

Added text that mentions it is for US-based organizations only.

@alexdevz alexdevz marked this pull request as ready for review October 19, 2025 21:22
@alexdevz alexdevz requested review from a team as code owners October 19, 2025 21:22
@alexdevz
Copy link
Member Author

let me know if i didnt do this right 😅

@alexdevz alexdevz changed the title Update eligibility criteria for premium pins Update eligibility criteria text for premium pins Oct 19, 2025
Copy link
Contributor

@Luke-Oldenburg Luke-Oldenburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably disable it using !event.country_US? and add a corresponding tooltip.

@alexdevz
Copy link
Member Author

We should probably disable it using !event.country_US? and add a corresponding tooltip.

Yep that would be smart, I just had no idea if we had something to flag for that.

@garyhtou
Copy link
Member

Good idea @Luke-Oldenburg, we can add that in another PR

@garyhtou garyhtou enabled auto-merge October 20, 2025 22:01
@garyhtou garyhtou dismissed Luke-Oldenburg’s stale review October 20, 2025 22:05

We'll tackle in another PR

@claude
Copy link

claude bot commented Oct 20, 2025

Code Review - PR #11883: Update eligibility criteria text for premium pins

Summary

This PR adds a US-only restriction notice to the premium pins perk due to high international shipping costs. The change is straightforward and addresses a business need.

Code Quality ✅

Positive aspects:

  • The change is minimal and focused on the specific issue
  • The text flows naturally with the existing copy
  • No syntax errors or formatting issues

Suggestions for improvement:

  1. Consider adding backend validation - Currently, the restriction is only communicated in the UI text. Consider adding server-side validation to prevent non-US organizations from submitting the form. This could be done by adding a check in the eligibility logic (lines 10-16 in app/views/events/perks/_premium_pins.html.erb) and validating the organization's country/location before allowing form submission.

  2. Update the ineligibility tooltip - The "Ineligible" button tooltip doesn't currently account for US-based restriction. Consider adding a new condition for non-US organizations.

  3. Consistency with similar perks - The Jukebox perk (app/views/events/perks/_premium_jukebox.html.erb:36) mentions international shipping costs aren't covered by the coupon. Consider if premium pins might benefit from a similar approach rather than full exclusion.

Potential Issues 🔍

Missing backend enforcement:

  • The fillout form (fillout_form on line 23) will still accept applications from international organizations since there's no programmatic check
  • Non-US teams could still click "Apply" if they meet the teenager count requirement
  • Consider adding logic to detect organization location and either disable the Apply button for non-US teams or add validation on form submission

User experience concern:

  • The restriction notice appears at the end of the promotional text, which means users read the exciting offer first before learning they may not be eligible
  • Consider rephrasing to mention the restriction earlier in the description

Security Concerns ✅

  • No security issues identified
  • The change is purely informational text

Performance Considerations ✅

  • No performance impact - this is a static text change

Test Coverage ⚠️

Missing tests:

  • No test files found for premium pins functionality
  • Consider adding tests to verify US-based organizations see and can access the perk, non-US organizations see appropriate messaging, and the eligibility logic works correctly

Recommendation: While this change is low-risk, the premium pins feature would benefit from test coverage, especially if backend validation is added.

Additional Recommendations

  1. Documentation: Consider updating any internal documentation about premium pins eligibility
  2. Consistent terminology: The PR body uses "US-based organizations" while the code says "US-based teams" - ensure consistency
  3. Temporary vs permanent: The wording "At the moment" suggests this might be temporary. Consider adding a TODO comment if this restriction should be revisited, or removing "At the moment" if this is a permanent policy change

Overall Assessment

Approved with suggestions

The change effectively communicates the new restriction to users. However, I recommend adding backend validation to prevent form submissions from ineligible organizations and improving the user experience by restructuring the message. These can be addressed in a follow-up PR if preferred.

Risk Level: Low
Recommended Action: Safe to merge as-is, with suggested improvements to be addressed in follow-up PRs

@garyhtou garyhtou added this pull request to the merge queue Oct 20, 2025
Merged via the queue into main with commit 082f298 Oct 20, 2025
14 checks passed
@garyhtou garyhtou deleted the alexdevz-patch-2 branch October 20, 2025 22:22
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2025
## Summary of the problem
<!-- Why are these changes being made? What problem does it solve? Link
any related issues to provide more details. -->
#11883 (comment)


## Describe your changes
<!-- Explain your thought process to the solution and provide a quick
summary of the changes. -->



<!-- If there are any visual changes, please attach images, videos, or
gifs. -->
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.

4 participants