Skip to content

Add function to Newsletters service to get logged in users email address - #259

Open
michaelclapham wants to merge 1 commit into
mainfrom
mc/newsletters/logged-in-email
Open

Add function to Newsletters service to get logged in users email address#259
michaelclapham wants to merge 1 commit into
mainfrom
mc/newsletters/logged-in-email

Conversation

@michaelclapham

@michaelclapham michaelclapham commented Jun 25, 2026

Copy link
Copy Markdown
Member

What does this change?

Adds new method to Newsletters service to get currently logged in user email so that newsletter sign up pages can auto-populate the email input textbox on the page.

How has this change been tested?

There are Android https://github.com/guardian/android-news-app/pull/13238 and iOS https://github.com/guardian/ios-live/pull/10072 PRs that provide an implementation of this new function by having created a pre-release version of bridget to test this new function. The implementations work when called manually from elsewhere in the code, but this PR needs to be built upon by the newsletters team to prove out the functionality end to end.

WebX Ticket: #261
AppX Ticket: https://app.asana.com/1/1210045093164357/project/1215309367148854/task/1216357574103304

How can we measure success?

Have we considered potential risks?

Images

Accessibility


@changeset-bot

changeset-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ab91b70

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@michaelclapham michaelclapham added the feature Departmental tracking: work on a new feature label Jun 25, 2026
@SiAdcock SiAdcock linked an issue Jul 31, 2026 that may be closed by this pull request
@michaelclapham
michaelclapham marked this pull request as ready for review August 3, 2026 11:41
@michaelclapham
michaelclapham requested review from a team as code owners August 3, 2026 11:41
Comment thread thrift/native.thrift
service Newsletters {
bool requestSignUp(1: string emailAddress, 2:string newsletterIdentityName)
bool requestSignUp(1: string emailAddress, 2:string newsletterIdentityName),
string getLoggedInUserEmail()

@michaelclapham michaelclapham Aug 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thrift question: For not signed in and error cases, what should we return from the native apps? Blank string? Null? Some sort of exception / error object? Something else? Does thrift have a concept of nulls/nullable/optional return types, or are all types capable of also returning null?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is okay, but not ideal. If you wanted to get a little fancier you could use something like this:

struct EmailResponse {
  1: optional string emailAddress
}

service Newsletters {
  EmailResponse getLoggedInUserEmail()
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We use the approach that Alex suggested above for Epics too:

struct MaybeEpic {
1: optional Epic epic;
}

Comment thread thrift/native.thrift
service Newsletters {
bool requestSignUp(1: string emailAddress, 2:string newsletterIdentityName)
bool requestSignUp(1: string emailAddress, 2:string newsletterIdentityName),
string getLoggedInUserEmail()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is okay, but not ideal. If you wanted to get a little fancier you could use something like this:

struct EmailResponse {
  1: optional string emailAddress
}

service Newsletters {
  EmailResponse getLoggedInUserEmail()
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Departmental tracking: work on a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New method: get logged in user's email address

3 participants