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

[BUG] After first_or_none of class FindMany, limit_number == 1 remains #1116

Open
eac0de opened this issue Jan 31, 2025 · 5 comments · May be fixed by #1117
Open

[BUG] After first_or_none of class FindMany, limit_number == 1 remains #1116

eac0de opened this issue Jan 31, 2025 · 5 comments · May be fixed by #1117
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@eac0de
Copy link

eac0de commented Jan 31, 2025

Describe the bug and reproduce and expected behavior
After first_or_none of class FindMany, limit_number == 1 remains.
So

  requests = RequestModel.find() # I have 3 requests 
  first_request = await requests.first_or_none() # Return first request
  requests_list = requests.to_list() # In my opinion It must to return 3 requests, but returns 1 after first_or_none because first_or_none  assigns limit_number == 1

It's very unobvious!

Solution

async def first_or_none(self) -> Optional[FindQueryResultType]:
    """
    Returns the first found element or None if no elements were found
    """
    existing_limit_number = self.limit_number
    res = await self.limit(1).to_list()
    self.limit_number = existing_limit_number
    if not res:
        return None
    return res[0]
@eac0de eac0de changed the title [BUG] After first_or_none of class FindMany, limit == 1 remains [BUG] After first_or_none of class FindMany, limit_number == 1 remains Jan 31, 2025
@staticxterm
Copy link
Contributor

Hi @eac0de, this is the expected behavior for this method. If there are 3 documents, it shall return the first one and limit the response to that single document that matches some search criteria.
This is mentioned in the documentation:
https://beanie-odm.dev/tutorial/finding-documents/#finding-documents
Is there something not clear in documentation you would like to propose to be changed?

@eac0de
Copy link
Author

eac0de commented Jan 31, 2025

Hi @eac0de, this is the expected behavior for this method. If there are 3 documents, it shall return the first one and limit the response to that single document that matches some search criteria.
This is mentioned in the documentation:
https://beanie-odm.dev/tutorial/finding-documents/#finding-documents
Is there something not clear in documentation you would like to propose to be changed?

Hi!
The documentation says that: "It returns the first found document or None, if no documents were found."

There is nothing written there about changing the object FindMany and when it is reused, bugs are created.This is a very inconvenient and non-obvious solution

@staticxterm
Copy link
Contributor

Apologies for misreading your example.
Indeed you're right about that, and it shouldn't mutate 'self' but return a new result object.
Will look more into this.

@staticxterm
Copy link
Contributor

Hi again @eac0de, good find. I like your proposed fix, would you mind opening a PR for this please?

@staticxterm staticxterm added bug Something isn't working good first issue Good for newcomers labels Jan 31, 2025
@eac0de
Copy link
Author

eac0de commented Jan 31, 2025

Hi again @eac0de, good find. I like your proposed fix, would you mind opening a PR for this please?

Hi, again)
Yes, sure)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants