fix(capabilities): implement a wrapper for capabilities #12208

Antreesy · 2024-04-25T13:56:26Z

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

No visual changes

🚧 Tasks

Implement logic to check if room is federated, to fetch remote capabilities
- Think of a way to store/ retrive remote capabilities
  As it's a request to server, it should be asyncronous. That means we must in some case to wait for remote capabilities or re-render application based on response (which is now isn't happen and requires a reload, if capability has changed
  Reload as we do for dirty hash
Implement helpers like hasFeature to replace long getCapabilities()?.spreed?.features?.includes('capability')
Distinguish between whether to consider local/remote capability (see ref issue)

🏁 Checklist

🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
🖥️ Tested with Desktop client or should not be risky for it
🖌️ Design was reviewed, approved or inspired by the design team
⛑️ Tests are included or not possible
📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

DorraJaouad

You can use store instead of browser storage. The purpose of keeping the last previous fetched capabilities is to check in case the new fetch renders a different list (on the same page) -> store is enough for that.

src/store/participantsStore.js

DorraJaouad · 2024-06-05T10:25:32Z

src/services/CapabilitiesManager.ts

+ public hasTalkFeature(token: 'local'|string, feature: string): boolean {
+ if (this.localCapabilities.spreed['features-local'].includes(feature)) {
+ return this.localCapabilities?.spreed?.features?.includes(feature) ?? false
+ }
+ return this.getCapabilities(token)?.spreed?.features?.includes(feature) ?? false
+ }


If the feature is local, we don't need to check remote server at all because those features are usually not affecting the chat, call etc (e.g: note-to-self)
If the feature is not flagged as local, both remote and local capabilities should be checked using AND (if any of the server doesn't have that feature, it should not be there).

Note: we need to be careful when flagging feature/config with local, I noticed that config => attachments => allowed is local whereas it is affecting the chat.

I'd agree that feature should be respected from both local / remote, if that's related to the conversation; remote user shouldn't have more possibilities, than he already has locally, and more than a local user:

Remote cap. Local cap. Resulting Example

🔴 🔴 🔴 Not conflicting

🟢 🟢 🟢 Not conflicting

🟢 🔴 🔴 "Moderator" from 29 can't ban participants, because he isn't aware of the feature

🔴 🟢 🔴 Can't upload attachments, if not allowed on remote server

But for the config, i think remote should be respected, unless we define it in config-local: as we have numeric and string values there, not only boolean

But for the config, i think remote should be respected, unless we define it in config-local: as we have numeric and string values there, not only boolean

For config, it's a bit tricky, maybe min(remote, local) for numeric config and for not measurable numeric (like integers to reflect modes), use remote, right ?
For now, chat-read-status (local) which should be following remote server, no ?

Or we treat each config individually for now.
chat-read-status differs per user and set up by him, if I'm not mistaken, so should be from local

chat-read-status (local) - On conversation API v3 and the chat API the last common read message is exposed which can be used to update the "read status" flag of own chat messages. The info should be shown only when the user also shares their read status. The user's value can be found in config => chat => read-privacy.

That means it is first configured by server (expose the last common read message) and then enabled/disabled by user.

chat-read-status isn't used in frontend code, so consider this out of scope of this PR. Let's stick with:

features should be respected on both

configs should be respected from remote, unless we don't define them as local || unless we don't set code to respect both individually for each case

Speaking of which, @nickvergessen :
config => attachments => allowed, shouldn't we respect both local and remote?
features => avatar, aren't we proxying the same local endpoint for avatar, so only local is enough?

yeah the docs might be from before we decided to proxy avatars.

As for attachments, I guess we should check both, but we can clarify once we implement them

src/components/RightSidebar/Participants/ParticipantsTab.vue

ShGKme

Looks very cool!

Just some implementation nitpicks and questions.

src/services/CapabilitiesManager.ts

ShGKme

Looks good to me ✨

- check talk-hash when joining federated conversation - keep and retrieve capabilities from BrowserStorage - distinguish cases for local and remote capabilities - implement 'hasTalkFeature' and 'getTalkConfig' helpers - respect local-only features and configs Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

…with conversation switch) Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

…ion related) Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

Antreesy · 2024-06-14T08:29:04Z

Rebased onto main and resolved conflicts (message components and composables)

DorraJaouad

Tested 🦅

src/components/ConversationSettings/ConversationSettingsDialog.vue

src/components/MediaSettings/MediaSettings.vue

src/components/ConversationSettings/ConversationSettingsDialog.vue

Antreesy added 2. developing feature: frontend 🖌️ "Web UI" client feature: federation 🌐 labels Apr 25, 2024

Antreesy added this to the 💙 Next Major (30) milestone Apr 25, 2024

Antreesy self-assigned this Apr 25, 2024

Antreesy force-pushed the feat/11850/capabilities-wrapper branch 2 times, most recently from e1c3b1f to 47bb67c Compare April 28, 2024 20:30

Antreesy requested review from DorraJaouad and ShGKme April 28, 2024 20:38

DorraJaouad reviewed Apr 30, 2024

View reviewed changes

src/store/participantsStore.js Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

Antreesy force-pushed the feat/11850/capabilities-wrapper branch from 47bb67c to 0bf4beb Compare May 8, 2024 16:53

Antreesy changed the base branch from main to feat/noid/add-local-capabilities May 8, 2024 16:54

Antreesy force-pushed the feat/11850/capabilities-wrapper branch from 0bf4beb to 77bc7dd Compare May 10, 2024 08:40

Base automatically changed from feat/noid/add-local-capabilities to main May 10, 2024 10:28

Antreesy mentioned this pull request May 10, 2024

fix(messageStore): adjust getters and tests #12318

Merged

3 tasks

Antreesy force-pushed the feat/11850/capabilities-wrapper branch from 77bc7dd to 326799e Compare May 15, 2024 09:16

Antreesy force-pushed the feat/11850/capabilities-wrapper branch from 326799e to 30650ac Compare May 29, 2024 14:09

Antreesy marked this pull request as ready for review May 29, 2024 14:12

DorraJaouad reviewed Jun 5, 2024

View reviewed changes

ShGKme reviewed Jun 5, 2024

View reviewed changes

ShGKme reviewed Jun 6, 2024

View reviewed changes

src/services/CapabilitiesManager.ts Outdated Show resolved Hide resolved

ShGKme approved these changes Jun 7, 2024

View reviewed changes

Antreesy added 4 commits June 14, 2024 10:27

feat(capabilities): apply manager for static capabilities (unchanged …

d9f80e4

…with conversation switch) Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

feat(capabilities): apply manager for dynamic capabilities (conversat…

62eea8b

…ion related) Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

test(capabilities): adjust tests for new changes

e3bdf72

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

Antreesy force-pushed the feat/11850/capabilities-wrapper branch from 36dff6f to e3bdf72 Compare June 14, 2024 08:27

DorraJaouad approved these changes Jun 14, 2024

View reviewed changes

src/components/ConversationSettings/ConversationSettingsDialog.vue Show resolved Hide resolved

src/components/MediaSettings/MediaSettings.vue Show resolved Hide resolved

src/components/ConversationSettings/ConversationSettingsDialog.vue Show resolved Hide resolved

Antreesy merged commit 617aa3a into main Jun 14, 2024
46 checks passed

Antreesy deleted the feat/11850/capabilities-wrapper branch June 14, 2024 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(capabilities): implement a wrapper for capabilities #12208

fix(capabilities): implement a wrapper for capabilities #12208

Antreesy commented Apr 25, 2024 •

edited

Loading

DorraJaouad left a comment

This comment was marked as resolved.

DorraJaouad Jun 5, 2024

Antreesy Jun 6, 2024 •

edited

Loading

DorraJaouad Jun 6, 2024

Antreesy Jun 6, 2024

DorraJaouad Jun 6, 2024

Antreesy Jun 7, 2024

nickvergessen Jun 13, 2024

ShGKme left a comment

ShGKme left a comment

Antreesy commented Jun 14, 2024

DorraJaouad left a comment

Remote cap.	Local cap.	Resulting	Example
🔴	🔴	🔴	Not conflicting
🟢	🟢	🟢	Not conflicting
🟢	🔴	🔴	"Moderator" from 29 can't ban participants, because he isn't aware of the feature
🔴	🟢	🔴	Can't upload attachments, if not allowed on remote server

fix(capabilities): implement a wrapper for capabilities #12208

fix(capabilities): implement a wrapper for capabilities #12208

Conversation

Antreesy commented Apr 25, 2024 • edited Loading

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🚧 Tasks

🏁 Checklist

DorraJaouad left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

DorraJaouad Jun 5, 2024

Choose a reason for hiding this comment

Antreesy Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

DorraJaouad Jun 6, 2024

Choose a reason for hiding this comment

Antreesy Jun 6, 2024

Choose a reason for hiding this comment

DorraJaouad Jun 6, 2024

Choose a reason for hiding this comment

Antreesy Jun 7, 2024

Choose a reason for hiding this comment

nickvergessen Jun 13, 2024

Choose a reason for hiding this comment

ShGKme left a comment

Choose a reason for hiding this comment

ShGKme left a comment

Choose a reason for hiding this comment

Antreesy commented Jun 14, 2024

DorraJaouad left a comment

Choose a reason for hiding this comment

Antreesy commented Apr 25, 2024 •

edited

Loading

Antreesy Jun 6, 2024 •

edited

Loading