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

Update send_message() README documentation to reflect ability to start a new chat #92

Conversation

elliot-100
Copy link
Collaborator

@olizimmermann this relates to your #62 - do you have any comments?

@elliot-100 elliot-100 changed the title Update send_message() signature in README; description in README and docstring Update send_message() README documentation to reflect ability to start a new chat Update send_message() README documentation to reflect ability to start a new chat Sep 14, 2023
@Olen
Copy link
Owner

Olen commented Sep 15, 2023

Just a question - is it possible to only specify user, or is group_id a requirement in the API?

@elliot-100
Copy link
Collaborator Author

elliot-100 commented Sep 15, 2023

Just a question - is it possible to only specify user, or is group_id a requirement in the API?

Implementation requires both:

elif group_uid is None or user is None:
    return {
        "error": "wrong usage, group_id and user_id needed or continue chat with chat_id"
        }

I don't think it should be necessary to supply group_id, as a user inherently belongs to one group.
I would prefer a definitive user_uid to be required here rather than a fuzzy user string that's internally looked up with get_person(user).

Also not sure why an actual Exception isn't raised.

But this amend is to correctly document the status quo.

@olizimmermann
Copy link
Contributor

I will push an update next week. Currently afk

@Olen
Copy link
Owner

Olen commented Sep 18, 2023

Yeah. We should merge this as it is only documentation. And the we can update the actual function later.

@Olen Olen merged commit 781b721 into Olen:main Sep 18, 2023
4 checks passed
@elliot-100 elliot-100 deleted the 91-in-readme-send_message-signature-doesnt-reflect-changes-made-in-0990 branch September 18, 2023 15:06
@olizimmermann
Copy link
Contributor

Hey guys,
Thanks for updating the docstring/readme.

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.

3 participants