-
Notifications
You must be signed in to change notification settings - Fork 35
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
Chat: automatic answers & translations #3051
Conversation
05ecad7
to
4e732b2
Compare
@@ -219,6 +219,7 @@ | |||
#: The url patterns of this module (see :doc:`django:topics/http/urls`) | |||
urlpatterns: list[URLPattern] = [ | |||
path("api/v3/regions/", include(region_api_urlpatterns)), | |||
path("api/v3/webhook/zammad/", user_chat.zammad_webhook, name="zammad_webhook"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note: /api/v3/webhook/...
is a generic new path for webhooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some documentation for that at https://github.com/digitalfabrik/integreat-cms/blob/develop/docs/src/api-docs.rst?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -417,6 +417,7 @@ class Region(AbstractBaseModel): | |||
max_length=256, | |||
blank=True, | |||
default="", | |||
unique=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot allow to regions to use the same Zammad server, because we need the reverse look up of the Zammad server IP during webhooks to get the region to which the Zammad belongs. One Zammad server per region is also good for privacy reasons as Zammad does not have a functionality to strictly separate regions (groups and roles are possible though), but admins are always global.
model_name="userchat", | ||
name="language", | ||
field=models.ForeignKey( | ||
default=3, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All existing chat which were created during the first test phase, will be assigned the German language. There is no other good solution.
f2582cd
to
dfcf067
Compare
Code Climate has analyzed commit 5c0112a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 55.7% (50% is the threshold). This pull request will bring the total coverage in the repository to 82.6% (-0.1% change). View more on Code Climate. |
678caec
to
1c447a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This hurts a bit, I guess 🙈 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a full review, just some questions I got from a quick look.
Also, is there / should there be any protection in place to avoid unauthorized people using this api?
@@ -219,6 +219,7 @@ | |||
#: The url patterns of this module (see :doc:`django:topics/http/urls`) | |||
urlpatterns: list[URLPattern] = [ | |||
path("api/v3/regions/", include(region_api_urlpatterns)), | |||
path("api/v3/webhook/zammad/", user_chat.zammad_webhook, name="zammad_webhook"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some documentation for that at https://github.com/digitalfabrik/integreat-cms/blob/develop/docs/src/api-docs.rst?
f8dcf79
to
d4fd16e
Compare
d4fd16e
to
5c0112a
Compare
Short description
Implement machine translations for chat & retrieval augmented answer generation
Proposed changes
Side effects
Resolved issues
Fixes: #3040
Pull Request Review Guidelines