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

DataStore documentation revamp Part 2/2 #1903

Merged
merged 36 commits into from
Jul 14, 2020

Conversation

drochetti
Copy link
Contributor

@drochetti drochetti commented Jun 3, 2020

Disclaimer: This is a work in progress PR. If you find anything bizarre like an iOS code snippet on an Android fragment, or clearly wrong code snippets it's because I'm still working on them (or I might just have missed them, who knows!?).

It has a dependency on #1827

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


<inline-fragment platform="js" src="~/lib/datastore/fragments/js/data-access/save-snippet.md"></inline-fragment>
<inline-fragment platform="ios" src="~/lib/datastore/fragments/ios/data-access/save-snippet.md"></inline-fragment>
<inline-fragment platform="android" src="~/lib/datastore/fragments/android/data-access/save-snippet.md"></inline-fragment>

The `save` method takes care of creating a new record or updating an existing one in case the model `id` already exists in the database.
Copy link
Contributor

Choose a reason for hiding this comment

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

The save method creates a new record, or in the event that one already exists in the database, it updates the record.

docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
### Migrations
## Migrations

Local migrations on device are not currently supported. If you are syncing with the cloud the structure and items of that data in your DynamoDB table will not be touched as part of this process.
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to describe what a local migration is before saying that it is not supported?

## Setup cloud sync

Synchronization between offline and online data can be tricky. DataStore goal is to remove that burden from the application code and handle all data consistency and reconciliation between local and remote behind the scenes, while developers focus on their application logic. Up to this point the focus was to setup a local datastore that works offline and has all the capabilities you would expect from a data persistence framework.
Copy link
Contributor

Choose a reason for hiding this comment

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

"up to this point" is ambiguous.... do we mean.. prior to launching amplify? or... something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the intent was to say up to that point in the docs (if you go page by page). A more clear sentence would definitely be better

Comment on lines 61 to 63

For instance, when updating or deleting data, one has to consider that the state of the local data might be outdated in case the data syncrhonization process
Copy link
Contributor

Choose a reason for hiding this comment

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

need to add periods to the end of these sentences to stay consistent.

### Conlfict detection and resolution

When concurrently updating the data in multiple places, it is likely that some conflict might happen. For most of the cases the default *Auto-merge* algorithm should be able to resolve. However, there are scenarios where the algorithm won't be able to solve and for those a more advanced option is available and will be described in details in the next section.
Copy link
Contributor

Choose a reason for hiding this comment

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

the second sentence ends abruptly. i think it should dread ".... should be able to resolve conflicts"
3rd sentence change: "won't be able to be solve and" to "won't be able to be resolved, and in these cases, a more advanced.."

<inline-fragment platform="android" src="~/lib/datastore/fragments/android/sync/40-clear.md"></inline-fragment>

This is a simple yet effective example. However, in a real scenario you might want to optimize it and only call `clear()` when a different user `signedIn` in order to avoid clearing the database for a repeated sign-in of the same user.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion:

  1. remove the words "optimize it and"
  2. I think we need to add the word "is", so that it reads: ".. when a different user is signedIn in order.."

@@ -0,0 +1 @@
TODO: add Android snippets
Copy link
Contributor Author

@drochetti drochetti Jun 4, 2020

Choose a reason for hiding this comment

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

@jpignata @jamesonwilliams heads up, will need help here

@@ -0,0 +1 @@
TODO: add Android snippets
Copy link
Contributor Author

@drochetti drochetti Jun 4, 2020

Choose a reason for hiding this comment

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

@jpignata @jamesonwilliams heads up, will need help here

@@ -0,0 +1 @@
// TODO add Android snippet
Copy link
Contributor Author

@drochetti drochetti Jun 4, 2020

Choose a reason for hiding this comment

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

@jpignata @jamesonwilliams heads up, will need help here

docs/lib/datastore/conflict.md Outdated Show resolved Hide resolved
docs/lib/datastore/conflict.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jun 15, 2020

This pull request introduces 4 alerts and fixes 1 when merging 30d7abc into 85728cb - view on LGTM.com

new alerts:

  • 4 for Unused variable, import, function or class

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2020

This pull request introduces 4 alerts and fixes 1 when merging 867bd57 into 85728cb - view on LGTM.com

new alerts:

  • 4 for Unused variable, import, function or class

fixed alerts:

  • 1 for Unused variable, import, function or class

Comment on lines +11 to +12
"how-it-works",
"examples"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dabit3 I broke apart "How it works" and the JS full app examples

</amplify-callout>

The code below illustrates a conflict resolution handler for the `Post` model that retries a mutation with the same title, but the most recent remote data for all other fields. The conflict resolution handler discards conflicts for all other models (by passing `.discard` to the `resolve` function).
The code below illustrates a conflict resolution handler for the `Post` model that retries a mutation with the same title, but the most recent remote data for all other fields. The conflict resolution handler discards conflicts for all other models (by passing `.applyRemote` to the `resolve` function).
Copy link
Contributor

Choose a reason for hiding this comment

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

on line 3, we say it can be specified in minutes, but it should be "seconds"

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, let me verify this before you take action on this.

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

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

A few comments, I need to look docs/lib/datastore/sync/md


To write any data to the DataStore you can pass an instance of a Model to `DataStore.save()` and it will be persisted in offline storage. At this point you can use it as an item in a normal data store such as querying, updating or deleting. If you choose to later connect to the cloud the item will be synchronized using GraphQL mutations and any other systems connected to the same backend can run queries or mutations on these items as well as observe them with GraphQL subscriptions.
To write data to the DataStore, pass an instance of a model to `Amplify.DataStore.save()`:
Copy link
Contributor

Choose a reason for hiding this comment

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

On JS is like it was mentioned before, maybe is needed inline-fragment for that as well

docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/data-access.md Outdated Show resolved Hide resolved
docs/lib/datastore/examples.md Show resolved Hide resolved
@j1mr10rd4n
Copy link
Contributor

#2105 has a fix for a typo in the relational save-snippet.

@drochetti drochetti merged commit 348d24b into next-datastore-getting-started Jul 14, 2020
@jpignata jpignata deleted the next-datastore-docs branch November 30, 2020 22:05
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.

6 participants