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

Update for Rules v2: remove "Device Sync Permissions" #2581

Merged
merged 2 commits into from Feb 22, 2023

Conversation

cbush
Copy link
Collaborator

@cbush cbush commented Feb 13, 2023

Pull Request Info

Jira

Staged Changes

Reminder Checklist

If your PR modifies the docs, you might need to also update some corresponding
pages. Check if completed or N/A.

  • Create Jira ticket for corresponding docs-app-services update(s), if any
  • Checked/updated Admin API
  • Checked/updated CLI reference

Review Guidelines

REVIEWING.md

@github-actions
Copy link

github-actions bot commented Feb 13, 2023

Readability for Commit Hash: 5885ea9

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

Flesch Reading Ease scores for changed documents:

  • source/sdk/flutter/sync/write-to-synced-realm: 58.21
  • source/sdk/swift/sync/write-to-synced-realm: 50.6

The following table can be helpful in assessing the readability score of a document.

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

@@ -23,31 +23,20 @@ The examples on this page use an Atlas App Services App with the following
Device Sync configuration and a client app with the following Realm SDK
data model and subscriptions.

Device Sync is configured with the following queryable fields:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed because queryable fields are now automatically determined.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought this was only when Development Mode is enabled?

If this is true when Development Mode is enabled, we should call that out instead of just removing this - i.e. "If Development Mode is enabled, Sync can automatically determine queryable fields from your Flexible Sync query. If Development Mode is not enabled, add these queryable fields."

Related, if this is true when Development Mode is enabled, then we can probably remove this info from all of the Quick Starts? I think they all have a bullet in the Device Sync prerequisites about configuring queryable field, but they also say to enable Development Mode, so this would be unnecessary there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Related, there is also mention of configuring queryable fields on the "Manage Flexible Sync Subscription" pages: https://www.mongodb.com/docs/realm/sdk/swift/sync/flexible-sync/#subscribe-to-queryable-fields

We might want to add something there about not needing to do this if Development Mode is enabled?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And in the "Add Device Sync to an App" page that exists in Swift: https://www.mongodb.com/docs/realm/sdk/swift/sync/add-sync-to-app/

Flutter has the same page but the verbiage there is slightly different so I don't think it needs any updates relative to queryable fields.

Copy link
Collaborator

@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.

The changes here look fine, except I have a Q about the queryable fields. If I've misunderstood, feel free to disregard - but if my understanding is correct, I'd suggest we tweak this update and also update some related places throughout the SDK docs.

@@ -23,31 +23,20 @@ The examples on this page use an Atlas App Services App with the following
Device Sync configuration and a client app with the following Realm SDK
data model and subscriptions.

Device Sync is configured with the following queryable fields:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought this was only when Development Mode is enabled?

If this is true when Development Mode is enabled, we should call that out instead of just removing this - i.e. "If Development Mode is enabled, Sync can automatically determine queryable fields from your Flexible Sync query. If Development Mode is not enabled, add these queryable fields."

Related, if this is true when Development Mode is enabled, then we can probably remove this info from all of the Quick Starts? I think they all have a bullet in the Device Sync prerequisites about configuring queryable field, but they also say to enable Development Mode, so this would be unnecessary there.

@@ -23,31 +23,20 @@ The examples on this page use an Atlas App Services App with the following
Device Sync configuration and a client app with the following Realm SDK
data model and subscriptions.

Device Sync is configured with the following queryable fields:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Related, there is also mention of configuring queryable fields on the "Manage Flexible Sync Subscription" pages: https://www.mongodb.com/docs/realm/sdk/swift/sync/flexible-sync/#subscribe-to-queryable-fields

We might want to add something there about not needing to do this if Development Mode is enabled?

@@ -23,31 +23,20 @@ The examples on this page use an Atlas App Services App with the following
Device Sync configuration and a client app with the following Realm SDK
data model and subscriptions.

Device Sync is configured with the following queryable fields:
Copy link
Collaborator

Choose a reason for hiding this comment

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

And in the "Add Device Sync to an App" page that exists in Swift: https://www.mongodb.com/docs/realm/sdk/swift/sync/add-sync-to-app/

Flutter has the same page but the verbiage there is slightly different so I don't think it needs any updates relative to queryable fields.

@cbush
Copy link
Collaborator Author

cbush commented Feb 13, 2023

@dacharyc you're right, the change to queryable fields only applies to Development Mode. I undid that change, and we'll address it in the proper ticket.

@cbush cbush merged commit d2ae02a into mongodb:master Feb 22, 2023
@cbush cbush deleted the permissions-v2 branch February 22, 2023 20:43
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.

None yet

2 participants