-
Notifications
You must be signed in to change notification settings - Fork 287
feat: add context chat provider #11150
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
base: main
Are you sure you want to change the base?
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. |
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. |
c2efb0b
to
6a236d3
Compare
dd566f9
to
a7bc7ba
Compare
52dcdfc
to
9077fa6
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.
Looks really good to me! If we can finish the tests, I'd look forward to a review from the groupware team 💙
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? |
f87d978
to
6023995
Compare
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
… chat import jobs Signed-off-by: Edward Ly <[email protected]>
… import jobs Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
- 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 <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
Signed-off-by: Edward Ly <[email protected]>
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? |
The import function should be called when the app registers itself with context chat for the first time, so it shouldn't matter which one is installed first. @ChristophWurst Did you install context_chat_backend as well? |
I currently cannot test this either, because (likely since this was rebased on main) the mail app in this branch cannot connect to any IMAP server. :/ |
Bingo! That one is missing |
Thanks to you and Alexander I have CCB running. I wasn't able to catch the initial import method with xdebug. Maybe it just hasn't run. I see this in CB stats:
How can I trigger that? |
mmh, this may be a bug in context_chat then 🤔 This method here should be called whenever an app interacts with context_chat: https://github.com/nextcloud/context_chat/blob/main/lib/Public/ContentManager.php#L95 and triggers an event for apps to register themselves. |
Mmh, looking into this it might be a mind-o that we hadn't realized so far: https://github.com/nextcloud/context_chat/blob/main/lib/Event/ContentProviderRegisterEvent.php#L10 context_chat triggers its own event. The thinking was that making this even inherít from the event in OCP would make it compatible, but since the class path is different, any app listening to the OCP event will likely never get called. |
This may be a fix, haven't tested yet: nextcloud/context_chat#187 |
No description provided.