Skip to content

feat(ocp): expose whether talk is enabled for user#59918

Open
hamza221 wants to merge 2 commits intomasterfrom
feat/check-if-talk-enabled
Open

feat(ocp): expose whether talk is enabled for user#59918
hamza221 wants to merge 2 commits intomasterfrom
feat/check-if-talk-enabled

Conversation

@hamza221
Copy link
Copy Markdown
Contributor

@hamza221 hamza221 commented Apr 26, 2026

Summary

Expose the Talk app admin settings start_conversations and allowed_groups through OCP

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI (Tests were AI generated)

Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
@hamza221 hamza221 requested a review from a team as a code owner April 26, 2026 15:08
@hamza221 hamza221 requested review from ArtificialOwl, artonge, leftybournes and provokateurin and removed request for a team April 26, 2026 15:08
@hamza221 hamza221 added this to the Nextcloud 34 milestone Apr 26, 2026
Comment on lines +59 to +62
* Check if Talk is disabled for the logged in user, respecting the
* per-group restriction configured in Talk admin settings
* (start_conversations).
*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* Check if Talk is disabled for the logged in user, respecting the
* per-group restriction configured in Talk admin settings
* (start_conversations).
*
* Check if Talk is disabled for the logged in user.
*

The idea of the abstraction in OCP is to not reference Talk details iirc.
So should also not reference internal app configs 🙈

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, while I know the config in Talk is like this. I would name it positively, so have a isEnabledForUser

Comment on lines +47 to +51
* Check if the logged in user is not allowed to create conversations,
* respecting the per-group restrictions configured in Talk admin settings
* (allowed_groups).
*
* Returns false when Talk is not available at all.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be the same here positive phrase and method name:

Suggested change
* Check if the logged in user is not allowed to create conversations,
* respecting the per-group restrictions configured in Talk admin settings
* (allowed_groups).
*
* Returns false when Talk is not available at all.
* Check if the logged in user is allowed to create conversations.

But while this is "trivial" at the moment, we have a pending request to allow "internal" conversations but block "public" conversations.
The question is whether we add a bool for public or the IConversationOptions as a parameter here.

But I guess from your calendar PR you don't know what kind of conversation the user is going to create? So maybe we keep it this way, and Talk returns true if a user is allowed to create any conversation here.
But creation call itself might still fail?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants