feat: add context chat provider #11150
Conversation
|
Thanks for opening your first pull request in this repository! ✌️ |
|
It was decided that we should move on with this without nextcloud/server#52852. I'll look for a way to make Psalm understand the foreign code. |
edward-ly
force-pushed
the
feat/context-chat
branch
2 times, most recently
from
16633b0 to
a359dd7
Compare
Since we have to do workarounds to get Psalm working with stubs, have a stub update mechanism, and additionally need the actual classes to be able to write tests, we decided that nextcloud/server#52852 should be pursued again. It's also an investment of time, but we'll have our standard setup for app communication through the OCP public API. That setup works for Psalm and PHPUnit. |
edward-ly
force-pushed
the
feat/context-chat
branch
2 times, most recently
from
c2efb0b to
6a236d3
Compare
edward-ly
force-pushed
the
feat/context-chat
branch
3 times, most recently
from
dd566f9 to
a7bc7ba
Compare
edward-ly
force-pushed
the
feat/context-chat
branch
8 times, most recently
from
52dcdfc to
9077fa6
Compare
edward-ly
force-pushed
the
feat/context-chat
branch
from
fbff09a to
5d992bb
Compare
| /** @var ITimeFactory */ | ||
| private $time; | ||
|
|
||
| private int $timestamp; |
There was a problem hiding this comment.
@marcelklehr does this need initialization or no?
There was a problem hiding this comment.
One of the previous test results failed on this
|
|
||
| public function testDelete(): void { | ||
| $this->insert(1, 1); | ||
| $this->insert(2, 2); |
There was a problem hiding this comment.
you insert mailbox_id=2, last_message_id=2 …
| $this->insert(1, 1); | ||
| $this->insert(2, 2); | ||
| $this->insert(3, 3); | ||
| $task = $this->service->delete(2); |
There was a problem hiding this comment.
… and here you delete primary id=2
that row does not exist most likely. I think what you want to do is use the mapper to insert the rows, then delete by the actual primary id.
edward-ly
force-pushed
the
feat/context-chat
branch
3 times, most recently
from
f2d2718 to
e983a66
Compare
|
I don't have a working app_api/assistant/context_chat setup and can only make a guess, but it seems that once an administrator sets up context_chat on a Nextcloud instance, new emails are automatically fed into that model? If that's correct, then we should have a per-account toggle to turn that off. |
edward-ly
force-pushed
the
feat/context-chat
branch
2 times, most recently
from
99e6c37 to
491dc95
Compare
I don't know if all of the existing apps have a similar toggle yet, but yeah, that's a good idea regardless. Where in the mail settings do you think would be a good place to put it? |
edward-ly
force-pushed
the
feat/context-chat
branch
2 times, most recently
from
f87d978 to
6023995
Compare
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
… chat import jobs Signed-off-by: Edward Ly <contact@edward.ly>
… import jobs Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
- rename Job to Task to make it easier to differentiate it from bg jobs - simplify db schema - change contentItem id to include mailbox id - move message filtering into DB queries - add lots of error handling Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
edward-ly
force-pushed
the
feat/context-chat
branch
from
6023995 to
15be331
Compare
|
@edward-ly could you please add instructions for how to test this? I set up context_chat and assistant. I ran cron many times. The initial import is not triggered. |
Hm, the docs say that "the triggerInitialImport method is called when Context Chat is first set up", but it's not clear what "set up" means, for sure. Maybe context_chat needs to be installed after the mail app so that it can find and trigger all existing providers? @marcelklehr do you know the specifics around this? |
5 participants
No description provided.