Possible error in the return of the update method

[MAD-py]

I have been creating the update_or_create method based on the get_or_create and update methods, doing some tests I found a possible bug in the update method.

in the database I have the following data.

[
  {
    "name": "Venom",
    "year": 2023
  },{
    "name": "Venom2",
    "year": 2021
  }
]

and when executing the next query Movie.query({Movie.name: "Venom"}).update({Movie.year: 2021}), what I would expect is for the method to return

[
  {
    "name": "Venom",
    "year": 2021
  }
]

But what it returns is

[
  {
    "name": "Venom",
    "year": 2021
  },{
    "name": "Venom2",
    "year": 2021
  }
]

Is this a bug or is expected to happen?

[aminalaee]

I agree this is not expected, I think this is a bug. Will you be able to take a look?

[MAD-py]

Of course, today is difficult for me, but I will take a look at it in the next few days.

[ischaojie]

is there really a need for update_or_create func?

[aminalaee]

Well the use case is different. Can you explain why you think it's not needed?

[MAD-py]

@aminalaee, could you explain why *args is used?

https://github.com/aminalaee/mongox/blob/e6698f62be29b00dc372884a82f7f0b07078f1ea/mongox/models.py#L207

and the error is because it is looking for the last updated field for the return and ignores the initial filter, I already have a possible solution but I want to understand the above first.

[aminalaee]

The args are the fields that need to be updated. More like a bulk update updating all matching criteria:

movies = await Movie.query(Movie.year == 1990).update(Movie.year == 2000)

We also have single object update (might not be related):

movie = await Movie.query().get()

movie.name = "Another Movie"
movie = await movie.save()

[MAD-py]

but it would be more like this

movies = await Movie.query({Movie.year: 1990}).update({Movie.year: 2000})

and if we want to update more fields it would be

movies = await Movie.query({Movie.year: 1990}).update({Movie.year: 2000, Movie.name: "Venom"})

if what we want is to be able to pass field to field it would be

movies = await Movie.query({Movie.year: 1990}).update(year=2000, name=Venom)

but for this we must use is **kwargs

[aminalaee]

I get your point now. Maybe we need to fix that in a separate PR? Probably we can fix update in a PR when we add test in the update case as you explained. I see the current test we have for update is not testing that https://github.com/aminalaee/mongox/blob/e6698f62be29b00dc372884a82f7f0b07078f1ea/tests/test_models.py#L252-L259

[MAD-py]

I agree that this should be a separate PR and it should be before creating the PR with the update_or_create method.

So, now I'll work on reorganize the update so then we can continue with update_or_create.

[aminalaee]

Great, Thank you @MAD-py

[MAD-py]

Issue resolved #29

[aminalaee]

@MAD-py Awesome, thank you. Do you think we could extend the tests for this, to make sure it works as expected? I mean if you're interested feel free, otherwise I can do that.

[MAD-py]

But this is already tested in these lines.

https://github.com/aminalaee/mongox/blob/c5161e53912d58320666033c079b0df9428dad31/tests/test_models.py#L264-L269

I have added those lines to the tests to support the solution of this bug.

[aminalaee]

Oh sorry my comment wasn't clear. I meant we found this when working with get_or_create so we could add it there. But you're right that's tested and not related here.

[MAD-py]

Ok, so that's all right?

[aminalaee]

Yeah perfect. Thanks