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

[fcmv1] Manage FCM V1 Google Service Account Key in CLI #2197

Merged

Conversation

christopherwalter
Copy link
Contributor

Why

NOTE: DO NOT LAND THIS UNTIL GRAPHQL CHANGES LAND IN WWW

We're adding support for FCM V1 credentials, as Google is shutting down the FCM Legacy API for sending Android notifications in June.

How

Add new prompts and refactor existing prompts to match this schematic:
eas-cli

Test Plan

Verified all functionality e2e:
fcm_v1_eas_cli_manage_creds

Copy link

linear bot commented Jan 23, 2024

Copy link

github-actions bot commented Jan 23, 2024

Size Change: +3.35 kB (0%)

Total Size: 51.3 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 51.3 MB +3.35 kB (0%)

compressed-size-action

Copy link
Member

@quinlanj quinlanj left a comment

Choose a reason for hiding this comment

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

I ran this version of eas-cli against your changes in www and the UX looks good. Thank you! Requesting changes for deduping classes 🙏

edit: When you are ready with the amended changes, please re-request review!

@quinlanj
Copy link
Member

Assigning actions

I agree with you here -- these actions makes sense to keep the submission and fcmv1 separate.
AssignGoogleServiceAccountKey - Let's rename this class to AssignGoogleServiceAccountKeyForSubmissions:

Create GSAK actions

There will be 3 'contexts' for creation:

  1. generic Upload a GSAK option
  2. setting up a submission GSAK (creates and assigns for submissions)
  3. setting up an FCM v1 GSAK (creates and assigns for FCMV1)

IMO the most important bit of logging here is this fyi bit. For the generic case, we'd want the fyi to link a generic page which lists the pages for submissions and fcmv1. For the specialized FCM and Submission cases, we'd want the fyi to lead to their respective instructions for the permissions the user needs to assign to that GSAK.

My vote is to refactor this one into a single class since there are 3 logging contexts, but the underlying logic is the same

SetUp GSAK actions

I'm ok with what you have here 👍

@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch from eb9152a to 813e46c Compare January 27, 2024 05:29
Copy link
Member

@szdziedzic szdziedzic left a comment

Choose a reason for hiding this comment

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

It looks reasonable to me 👍

Thanks for sharing the video from e2e testing the change and thanks for taking care of it 🙌

@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch from 813e46c to b096105 Compare February 7, 2024 13:30
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 7, 2024
# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 7, 2024
# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good
@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch from b096105 to 44df163 Compare February 7, 2024 15:01
@christopherwalter
Copy link
Contributor Author

IMO the most important bit of logging here is this fyi bit. For the generic case, we'd want the fyi to link a generic page which lists the pages for submissions and fcmv1. For the specialized FCM and Submission cases, we'd want the fyi to lead to their respective instructions for the permissions the user needs to assign to that GSAK.

@quinlanj I removed CreateGoogleServiceAccountKeyForFcmV1 and updated the logging in CreateGoogleServiceAccountKey to include language about both the submission use case as well as the notifications use case. I also have this PR up to update the FYI docs for the two use cases.

Let me know what you think.

@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch 3 times, most recently from 99510f4 to 91a1a29 Compare February 7, 2024 21:37
@christopherwalter
Copy link
Contributor Author

/changelog-entry new-feature Support configuring a Google Service Account Key via eas credentials, for sending Android Notifications via FCM V1

@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch 2 times, most recently from ff02983 to 2090787 Compare February 7, 2024 23:15
# Why
NOTE: DO NOT LAND THIS UNTIL GRAPHQL CHANGES LAND IN WWW

We're adding support for FCM V1 credentials, as Google is shutting down
the FCM Legacy API for sending Android notifications in June.

# How
Add new prompts and refactor existing prompts to match this schematic:

# Test Plan
Verified all functionality e2e:
@christopherwalter christopherwalter force-pushed the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch from 2090787 to fc1461b Compare February 7, 2024 23:16
@christopherwalter
Copy link
Contributor Author

/changelog-entry new-feature Support configuring a Google Service Account Key via eas credentials, for sending Android Notifications via FCM V1

Copy link

github-actions bot commented Feb 7, 2024

✅ Thank you for adding the changelog entry!

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 60 lines in your changes are missing coverage. Please review.

Comparison is base (0b71812) 54.16% compared to head (66452b9) 54.08%.

Files Patch % Lines
...id/actions/SetUpGoogleServiceAccountKeyForFcmV1.ts 24.25% 22 Missing and 3 partials ⚠️
...s/eas-cli/src/credentials/manager/ManageAndroid.ts 16.67% 19 Missing and 1 partial ⚠️
...d/actions/AssignGoogleServiceAccountKeyForFcmV1.ts 25.00% 6 Missing ⚠️
...graphql/mutations/AndroidAppCredentialsMutation.ts 0.00% 4 Missing ⚠️
.../src/credentials/android/utils/printCredentials.ts 75.00% 3 Missing ⚠️
...s-cli/src/credentials/android/api/GraphqlClient.ts 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2197      +/-   ##
==========================================
- Coverage   54.16%   54.08%   -0.08%     
==========================================
  Files         516      518       +2     
  Lines       18814    18894      +80     
  Branches     3969     3984      +15     
==========================================
+ Hits        10188    10216      +28     
- Misses       7934     7981      +47     
- Partials      692      697       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christopherwalter christopherwalter merged commit a5c1100 into main Feb 7, 2024
9 checks passed
@christopherwalter christopherwalter deleted the chriswalter/eng-11172-fcmv1-specify-creds-in-eas-cli branch February 7, 2024 23:26
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 8, 2024
# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 8, 2024
# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 8, 2024
# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good
christopherwalter added a commit to expo/fyi that referenced this pull request Feb 9, 2024
#146)

* [1/1][docs] Add FCM V1 instructions to Google Service Account Key docs

# Why
We need to amend these docs to support using a Google Service Account Key for sending Android notifications via FCM V1. These docs are linked in the CLI when users are configuring Google Service Account Keys for submissions and notifications. (See @quinlanj's comment on this PR in eas-cli: expo/eas-cli#2197 (comment)).

# How
1. Update the top-level intro with an explanation of the two use cases for uploading a Google Service Account Key to EAS.
2. Move existing instructions for `eas submit` into a new subsection
3. Add a subsection for instructions for uploading GSAK for FCM V1

# Test Plan
Get feedback on copy, check that preview markdown looks good

* Update creating-google-service-account.md

Co-authored-by: Aman Mittal <[email protected]>

---------

Co-authored-by: Aman Mittal <[email protected]>
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.

3 participants