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

GraphQL should not return _deleted items in response #12933

Closed
3 tasks done
osehmathias opened this issue Jan 31, 2024 · 5 comments
Closed
3 tasks done

GraphQL should not return _deleted items in response #12933

osehmathias opened this issue Jan 31, 2024 · 5 comments
Assignees
Labels
GraphQL Related to GraphQL API issues question General question

Comments

@osehmathias
Copy link

osehmathias commented Jan 31, 2024

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

GraphQL API, DataStore

Amplify Version

v6

Amplify Categories

auth, storage, function, api, analytics, hosting, notifications

Backend

Amplify CLI

Environment information

Not relevant.

Describe the bug

DataStore is fantastic. GraphQL is fantastic. When they work together, they are not fantastic.

Sometimes, you need to use both together. For example, when you have a React app that uses GraphQL, where it is not appropriate to use DataStore because it syncs all the user's data to the device, unencrypted, which is not allowed for health applications. For example, when you have a Flutter app that uses DataStore, where it is fine to use DataStore because the app instance is encrypted, but not appropriate to use GraphQL given the latency issues in fetching data do not make for a great mobile experience.

These two systems should be able to work together closely.

There are two major problems.

  1. The versioning. On every update, you have to fetch the latest version / store the latest version in state, then increment the latest version based on the response. Not elegant, but doable.
  2. _deleted - this one is REALLY bad. You can't filter on the field, and even if you delete an item, it has no effect on the GraphQL api which really should not be returning deleted items. I believe this is a relatively simple fix for the team. In lieu, I have to update the resolvers or add some funky frontend stuff with a custom field. It's not great.

Expected behavior

_deleted records do not get fetched with the GraphQL API.

Reproduction steps

  1. Set up versioning for datastore, as you might do when you have a mobile app
  2. For this same backend, create a web app with GraphQL
  3. Delete a record
  4. Observe your deleted record floating about willy nilly in your React app, with no obvious way to get rid of it
  5. Shed a small tear and think about how you can help the community to solve this problem
@cwomack
Copy link
Member

cwomack commented Feb 1, 2024

Hello, @osehmathias 👋 and thank you for opening this issue. Want to see if I can help provide some guidance on what you're experiencing and see if there's a way to get the desired outcome you're looking for.

It's expected behavior for the soft-deleted records to be included when performing GraphQL operations against a sync-enabled API. As of 05/02/23 when this PR was merged into the amplify-category-api repo, you should be able to filter soft deleted-records out.

Can you clarify if you might be using an older version of the CLI and/or if you haven't potentially pushed a change to your API (via the amplify push CLI command and selecting GraphQL) since we added this feature?

@cwomack cwomack self-assigned this Feb 1, 2024
@cwomack cwomack added pending-response question General question and removed DataStore Related to DataStore category labels Feb 1, 2024
@osehmathias
Copy link
Author

osehmathias commented Feb 1, 2024

@cwomack

My amplify version is as follows:

amplify --version

12.10.1

I started the project after the release date of said feature, and have been constantly pushing updates the API. I started with GraphQL and later added DataStore.

My problem is not with DataStore.

When I delete a record through the GraphQL API, e.g.

await API.graphql(graphqlOperation(deleteRecord, { input: { id } } ) ) 

The response comes back with a _deleted: true, but the record is still returned in queries.

Unfortunately, I was not able to update the resolver as I can't filter on a non-indexed field, unless I define it in the schema, and even then, I don't think you can use boolean filters in Amplify.

I ended up just filtering out all deleted records on the frontend after the query. It's not ideal because of pagination.

@chrisbonifacio
Copy link
Member

chrisbonifacio commented Feb 2, 2024

@osehmathias This is expected behavior. You should be able to filter on the _deleted field on the server-side without explicitly adding it to your schema as @cwomack mentioned.

If you look at the amplify/backend/api/<api-name>/build/schema.graphql file you should be able to find filter input types like the following:

input ModelPostFilterInput {
  id: ModelIDInput
  title: ModelStringInput
  and: [ModelPostFilterInput]
  or: [ModelPostFilterInput]
  not: ModelPostFilterInput
  _deleted: ModelBooleanInput
  blogPostsId: ModelIDInput
}

you can then filter in your query like this:

query MyQuery {
  listPosts(filter: {_deleted: {attributeExists: false}}) {
    items {
      id
      title
      _deleted
    }
  }

OR

query MyQuery {
listPosts(filter: {_deleted: {ne: true}}) {
  items {
    id
    title
    _deleted
  }
}

Closing this issue as it is expected behavior and probably not going to change after the _deleted field has been exposed via the filter input. We can re-open if you experience issues with filtering.

We don't recommend mixing DataStore and the API GraphQL client in the same app but if you have to:
Just in case if you haven't run into it already, one other thing to watch out for when using the graphql client with datastore enabled is that mutations will only work if you pass in the current _version of the record in the input. If you pass in an old version the record doesn't update, you will receive the record back with the existing data. If you pass in a _version greater than the current server version you will receive an error saying as much.

@chrisbonifacio chrisbonifacio self-assigned this Feb 2, 2024
@chrisbonifacio chrisbonifacio removed the pending-triage Issue is pending triage label Feb 2, 2024
@osehmathias
Copy link
Author

osehmathias commented Feb 5, 2024

listPosts(filter: {_deleted: {attributeExists: false}})

The _version is manageable. With a debounce and a fetch prior to update, it works fine.

I know using GraphQL API and DataStore together is not recommended, but in this instance I can't use DataStore in the web version.

@OperationalFallacy
Copy link

@chrisbonifacio

don't recommend mixing DataStore and the API GraphQL client in the same app

I'm mixing it too as many other devs, since there is no DataStore client for the backend 🤷

The front-end would be a pita without it, and the front-end is pita without custom queries in Lambda (backend).

Regarding the workaround with the filter, it wouldn't work for a simple case of getting the latest stored item (schema has index with sortKeyFields of time created)

      const queryVariables: InternalStateByCreationTimeQueryVariables = {
        internalState: INTERNAL_STATE.CREATED,
        limit: 1,
        sortDirection: ModelSortDirection.DESC,
      };

Unless DDB expiration is set to a value much lower than the frequency of the access pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GraphQL Related to GraphQL API issues question General question
Projects
None yet
Development

No branches or pull requests

5 participants