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.

Sign up for GitHub

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

Jump to bottom

Support any type of id #110

Merged
merged 20 commits into from
Jun 25, 2024
Merged

Support any type of id #110

merged 20 commits into from
Jun 25, 2024

Conversation

amandasavluchinske
Copy link
Contributor

@amandasavluchinske amandasavluchinske commented Jun 21, 2024 •
edited
Loading

Closes #51

fjsj
fjsj reviewed Jun 21, 2024
View reviewed changes
django_ai_assistant/conf.py Outdated Show resolved Hide resolved
fjsj
fjsj reviewed Jun 21, 2024
View reviewed changes
django_ai_assistant/models.py Outdated Show resolved Hide resolved
fjsj
fjsj reviewed Jun 21, 2024
View reviewed changes
frontend/openapi_schema.json Outdated Show resolved Hide resolved
fjsj
fjsj reviewed Jun 21, 2024
View reviewed changes
tests/settings.py Outdated Show resolved Hide resolved
fjsj
fjsj requested changes Jun 21, 2024
View reviewed changes
Copy link
Member

@fjsj fjsj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it's on the right direction. Please check comments, I think we can simplify this.

@amandasavluchinske
Copy link
Contributor Author

Thanks! I'll work on that today.

@amandasavluchinske amandasavluchinske marked this pull request as ready for review June 21, 2024 19:55
@amandasavluchinske amandasavluchinske changed the title RFC (draft code): support any type of id Support any type of id Jun 21, 2024
@amandasavluchinske amandasavluchinske requested a review from fjsj June 21, 2024 19:59
fjsj
fjsj reviewed Jun 21, 2024
View reviewed changes
django_ai_assistant/helpers/use_cases.py
@@ -287,4 +287,6 @@ def delete_message(
if not can_delete_message(message=message, user=user, request=request):
raise AIUserNotAllowedError("User is not allowed to delete this message")

return DjangoChatMessageHistory(thread_id=message.thread_id).remove_messages([str(message.id)])
return DjangoChatMessageHistory(thread_id=message.thread_id).remove_messages(
message_ids=[str(message.id)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I think the str conversion is not needed anymore. Can you check that?

fjsj
fjsj approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member

@fjsj fjsj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but it's good if @pamella reviews too

pamella
pamella approved these changes Jun 25, 2024
View reviewed changes
Copy link
Collaborator

@pamella pamella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

django_ai_assistant/decorators.py Show resolved Hide resolved
@fjsj fjsj merged commit 73503db into main Jun 25, 2024
11 checks passed
@fjsj fjsj deleted the feat/support-any-type-of-id branch June 25, 2024 14:58
@amandasavluchinske
Copy link
Contributor Author

Oops, I was still testing haha

fjsj added a commit that referenced this pull request Jun 28, 2024
@fjsj
Merge pull request #110 from vintasoftware/feat/support-any-type-of-id
4391adb 
Support any type of id
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pamella pamella pamella approved these changes

@fjsj fjsj fjsj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support any type of ID field for user and our models in our views and functions
3 participants
@amandasavluchinske @fjsj @pamella