Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(relay): Show subscriptions ToS and PP for service=relay #18084

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

LZoog
Copy link
Contributor

@LZoog LZoog commented Dec 3, 2024

Because:

  • Relay has requested we display these ToS and PP for the service=relay browser integration

This commit:

  • Adjusts content-server index page, and adjust settings to display the same links and text as we do for Monitor

closes FXA-10815


See this thread where I got the thumbs up to display what we are displaying for Monitor.

@LZoog LZoog requested a review from a team as a code owner December 3, 2024 21:54
Because:
* Relay has requested we display these ToS and PP for the service=relay browser integration

This commit:
* Adjusts content-server index page, and adjust settings to display the same links and text as we do for Monitor

closes FXA-10815
Copy link
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

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

LGTM. To confirm, we'll still be adding terms for web relay separately even though it will be the same links?

expect(linkElements[3]).toHaveAttribute('href', '/legal/privacy');
it('renders component as expected when service=relay', () => {
renderWithLocalizationProvider(<TermsPrivacyAgreement isDesktopRelay />);
// testAllL10n(screen, bundle);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we could probably remove this commented out code, it's unclear when we'll get to setting up these tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but it's scattered throughout this test and other files so I think I will leave this for now to avoid another CI run 😅

@LZoog
Copy link
Contributor Author

LZoog commented Dec 4, 2024

LGTM. To confirm, we'll still be adding terms for web relay separately even though it will be the same links?

Correct (at least I assume the web version will show subscription ToS as well) - for this flow, we have that handy isDesktopRelay check. For the web one, since we need to add their client ID to our list of "special case" RPs, I recommended we wait until we work on that database query issue linked on that ticket, but I won't be surprised if we end up doing that one before we do the DB query.

@LZoog LZoog merged commit 50a0d57 into main Dec 4, 2024
26 checks passed
@LZoog LZoog deleted the FXA-10815 branch December 4, 2024 16:47
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.

2 participants