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

Add X-Source header to Gravatar REST API request #368

Merged
merged 5 commits into from
Oct 8, 2024

Conversation

hamorillo
Copy link
Contributor

Closes #360

Description

This is a follow-up of #353.

Having the request's source can help understand how developers use the SDK and their needs.

If the app provides the SDK with the context through the Gravatar object, we'll use that context to extract the application's name and send it in the X-Source header.

Currently, we only use the Context for that purpose; it's optional to provide it. When it's not provided, we won't send the header.

Testing Steps

  1. Navigate to the QE tab in the demo-app
  2. Switch through different avatars (Any QE request should work).
  3. With the Network Inspector, verify you can see the header
image
  1. In the demo-app code, remove the context from the initialization and run the demo-app
// Initialize the Gravatar SDK with the API key if it is available
BuildConfig.DEMO_GRAVATAR_API_KEY?.let { Gravatar.apiKey(it) }
  1. Navigate to the QE tab in the demo-app
  2. Switch through different avatars
  3. With the Network Inspector, verify you CAN'T see the header
  4. Everything else works as expected.

@hamorillo hamorillo added enhancement Enhancement :gravatar Gravatar core module labels Oct 8, 2024
@hamorillo hamorillo added this to the 2.0.0 milestone Oct 8, 2024
@hamorillo hamorillo force-pushed the hamorillo/360-header-app-name branch from 8f7307a to 7ce041f Compare October 8, 2024 10:57
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 8, 2024

📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
App Name Gravatar Demo
Commit37d4926
Direct Downloadgravatar-demo-prototype-build-pr368-37d4926.apk

Copy link
Contributor

@AdamGrzybkowski AdamGrzybkowski left a comment

Choose a reason for hiding this comment

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

Works as described 👍 Left two minor comments.

@@ -63,7 +63,7 @@ android {
Then you can access the API key in your app's code like this:

```kotlin
Gravatar.initialize(BuildConfig.GRAVATAR_API_KEY)
Gravatar.apiKey(BuildConfig.GRAVATAR_API_KEY).context(appContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Gravatar.apiKey(BuildConfig.GRAVATAR_API_KEY).context(appContext)
Gravatar.apiKey(BuildConfig.GRAVATAR_API_KEY)
.context(appContext) // Optional

Should we add a comment that the context is optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 What about:

// Optional but highly encouraged.

As it provides valuable information for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

hamorillo and others added 5 commits October 8, 2024 14:20
If the context is provided, we will extract the app name to be used in the request headers.
When the appName is available, we should send the X-Source header with that value.
I would say it should work with any context. But let's change it to prevent issues.

Co-authored-by: Adam Grzybkowski <[email protected]>
@hamorillo hamorillo force-pushed the hamorillo/360-header-app-name branch from c2ae693 to 37d4926 Compare October 8, 2024 12:20
@hamorillo hamorillo merged commit 388df8f into trunk Oct 8, 2024
16 checks passed
@hamorillo hamorillo deleted the hamorillo/360-header-app-name branch October 8, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement :gravatar Gravatar core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "X-source=app name" tag to API requests
3 participants