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

(DOCSP-33539): React Native: Update Create & Manage User API Keys to use auth hooks #3180

Merged
merged 16 commits into from
Jul 12, 2024

Conversation

dacharyc
Copy link
Collaborator

@dacharyc dacharyc commented Feb 13, 2024

Pull Request Info

Jira ticket: https://jira.mongodb.org/browse/DOCSP-33539

This PR adds to the React Native test suite, enabling us to test API user auth in a more robust way. The React Native tests in general are still somewhat flaky in CI - some of them are failing the CI for this PR but pass locally. These are mostly related to tests that make network requests. The pattern established in this PR should help address network issues and make tests more reliable in CI.

Essentially, the test is more an integration test than a unit test. I recommend we go to this higher-level testing for most networking aspects for examples that integrate with UI elements.

Reminder Checklist

Before merging your PR, make sure to check a few things.

  • Did you tag pages appropriately?
    • genre
    • meta.keywords
    • meta.description
  • Describe your PR's changes in the Release Notes section
  • Create a Jira ticket for related docs-app-services work, if any

Release Notes

  • React Native SDK
    • Manage Users/Authenticate Users: Update information and code example for logging in with an API key.
    • Manage Users/Create & Manage User API Keys: Update information and code examples for creating, deleting, enabling, and disabling API keys from the client.

Review Guidelines

REVIEWING.md

dacharyc and others added 5 commits February 13, 2024 13:56
@krollins-mdb
Copy link
Collaborator

krollins-mdb commented May 17, 2024

Currently getting the following error or one about no refresh token on the last bit of the test.

http error code considered fatal. Client Error: 403

This only happens in the Jest test. Everything works in the simulator. Trying to figure it out.

Copy link

netlify bot commented Jul 5, 2024

Deploy Preview for device-sdk ready!

Name Link
🔨 Latest commit fdda3b4
🔍 Latest deploy log https://app.netlify.com/sites/device-sdk/deploys/66913c3fecdfe30007bb5be9
😎 Deploy Preview https://deploy-preview-3180--device-sdk.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Jul 5, 2024

Readability for Commit Hash: fdda3b4

You can see any previous Readability scores (if they exist) by looking
at the comment's history.

Readability scores for changed documents:

  • source/sdk/react-native/manage-users/manage-user-api-keys: Grade Level: 5.8, Reading Ease: 75.61
  • source/sdk/react-native/manage-users/authenticate-users: Grade Level: 9.1, Reading Ease: 50.94

For Grade Level, aim for 8 or below.

For Reading Ease scores, aim for 60 or above:

Score Difficulty
90-100 Very Easy
80-89 Easy
70-79 Fairly Easy
60-69 Medium
50-59 Fairly Hard
30-49 Hard
0-29 Very Hard

For help improving readability, try Hemingway App.

@krollins-mdb krollins-mdb marked this pull request as ready for review July 5, 2024 19:19
Copy link
Collaborator Author

@dacharyc dacharyc left a comment

Choose a reason for hiding this comment

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

I think the new approach re: the steps is clever! But I don't love that this creates a dependency chain from step N to step N. We have been trying to move to making our tests more separated, and eliminate inter-dependencies - so it seems like this is moving in the opposite direction. Maybe we could talk offline about this shift in approach?

client accessed with an authenticated user's :js-sdk:`User.apiKeys
<Realm.User.html#apiKeys>` property.
<classes/Realm.User.html#apiKeys>` property.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I would like to see a heading between this intro bit and the @realm/react info. It would be nice to have this as a scannable heading in the OTP. i.e. something like:

API Key Auth with @realm/react

Or:

React Hooks

Or something that indicates this is the info about how to use this with hooks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense. Thank you for pointing this out!

Copy link
Collaborator

@krollins-mdb krollins-mdb left a comment

Choose a reason for hiding this comment

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

Dachary has reviewed and approved, but since this was originally her PR, she can't approve it on GitHub. So, I'm approving in her place. 😄

@krollins-mdb krollins-mdb merged commit cbe9d61 into mongodb:master Jul 12, 2024
10 of 11 checks passed
@docs-builder-bot
Copy link

@krollins-mdb krollins-mdb added the Add to consolidation feature branch PR merged after cutting the consolidation feature branch, so cherry-pick to the feature branch. label Jul 15, 2024
@dacharyc dacharyc removed the Add to consolidation feature branch PR merged after cutting the consolidation feature branch, so cherry-pick to the feature branch. label Jul 19, 2024
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