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

Queries including a toOne relationship that is soft deleted is not being excluded on query result #3

Open
leandroromano opened this issue May 11, 2023 · 3 comments

Comments

@leandroromano
Copy link

leandroromano commented May 11, 2023

Hi @olivierwilkinson !

First of all, thank you so much for this library! It is really very useful 🙌🏻

I have a quick question: I'm trying to do a query where a nested field that is soft deleted is not being excluded from the query and It is returned on the result. From what I understood on your docs, this nested field should be excluded, right? Specifically this quote of your docs

Models configured to use soft delete that are related to other models through a toOne relationship must have this relationship defined as optional. This is because the middleware will exclude soft deleted records when the relationship is included or selected. If the relationship is not optional the types for the relation will be incorrect and you may get runtime errors.

For example if you have an author relationship on the Comment model and the User model is configured to use soft delete you would need to change the relationship to be optional

If It helps, this is my schema.prisma entities related to this:

model UserAddress {
  id              String          @id @default(uuid())
  name            String
  user            User?           @relation(fields: [userId], references: [id])
  userId          String?
}

model User {
  id                                String                     @id
  email                             String?                    @unique
  name                              String?
  birthdate                         DateTime?
  createdAt                         DateTime                   @default(now())
  deletedAt                         DateTime?
  addresses                         UserAddress[]
}

And my stack is RedwoodJS as fullstack framework and I'm doing the following query:

query {
    userAddresses {
        name
        user {
            email
            deletedAt
        }
    }
}

That under the hood in the resolver it is executing this prisma query:

export const userAddresses: QueryResolvers['userAddresses'] = () => {
  return db.userAddress.findMany();
};

And as i said, it is returning a soft deleted user. But maybe i didn't understand the behaviour of toOne relationships, but from what I understood this should return something like this:

{
    "data": {
        "userAddresses": [
            {
                "name": "Test address"
                "user": {
                    "email": "[email protected]",
                    "deletedAt": null
                }
            },
            {
                "name": "Test address 2"
                "user": null //Being this the soft deleted user record?
            }
        ]
    }
}

Thanks in advance!

@olivierwilkinson
Copy link
Owner

Heya 👋 ,

I'm glad you are finding the library useful! Sorry I've been slow responding to this, I've not used RedwoodJS so needed to look into it a little bit.

So for the example above it looks like the middleware is behaving as expected since the user that has been soft-deleted is excluded from the results. You are right that the null value is the soft-deleted user, if there was a user object with "deletedAt": [Date] then the middleware would have failed to exclude it. To exclude userAddresses that don't have a user I think you would need to change the Prisma query being run to do that. If you would like an example for that let me know and I can try to help there / add one to the docs 😁

Having said all that when looking into RedwoodJS I saw that they are using the Fluent API for including relations by default. I've not explicitly tested the Fluent API with the middleware so there may be some unexpected behaviour. I'll spend some time adding tests the Fluent API soon but until I've done that I'll leave this open in-case I find some issues that you should be aware of!

Thanks for trying the library, please don't hesitate to let me know if you find any other issues or confusing behaviour 🙏

@leandroromano
Copy link
Author

leandroromano commented May 19, 2023

Hey!

Thank you for your response, and there is no problem with the delay! I really appreciate that you took a look into RedwoodJS to give me a more detailed answer

I think I wasn't very clear in explaining the error because this is the current output:

{
    "data": {
        "userAddresses": [
            {
                "name": "Test address"
                "user": {
                    "email": "[email protected]",
                    "deletedAt": null
                }
            },
            {
                "name": "Test address 2"
                "user": {
                    "email": "[email protected]",
                    "deletedAt": "2023-05-10"
                }
            }
        ]
    }
}

I believe this should be the expected output:

{
    "data": {
        "userAddresses": [
            {
                "name": "Test address"
                "user": {
                    "email": "[email protected]",
                    "deletedAt": null
                }
            },
            {
                "name": "Test address 2"
                "user": null 
            }
        ]
    }
}

Am I right in saying that the current result is wrong? Because the second user is actually soft deleted, so I think there might be a bug in queries that include a toOne relationship, where soft-deleted entries are not being excluded from the query result. This error is not present in queries where a one-to-many relationship is included because the soft-deleted entities are successfully filtered out

Thanks again, and sorry for the confusion in the issue description initially

@olivierwilkinson
Copy link
Owner

Ah! Yes then the middleware is not working as expected, most likely because the Fluent API needs some extra work to support.

Thank you for bringing this to my attention! And no worries at all about initial description 😁

I'm not sure what it will take to fix this unfortunately, but I will look into it and get back to you with an update soon 👍

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

No branches or pull requests

2 participants