Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Add a customisation point for widget permissions to preapprove embedding#9931

Closed
maheichyk wants to merge 1 commit intomatrix-org:developfrom
nordeck:widget_permissions_preapproved_embedding_customization
Closed

Add a customisation point for widget permissions to preapprove embedding#9931
maheichyk wants to merge 1 commit intomatrix-org:developfrom
nordeck:widget_permissions_preapproved_embedding_customization

Conversation

@maheichyk
Copy link
Contributor

@maheichyk maheichyk commented Jan 18, 2023

A new customization point for widget permissions to allow preapproval of embeddings for the widgets.

This will allow forks customization and consistent with #9910, #5439

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

  • Add a customisation point for widget permissions to preapprove embedding (#9931). Contributed by @maheichyk.

@maheichyk maheichyk requested a review from a team as a code owner January 18, 2023 14:58
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Jan 18, 2023
@maheichyk
Copy link
Contributor Author

Signed-off-by: Mikhail Aheichyk mikhail.aheichyk@nordeck.net

@dbkr dbkr added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Jan 18, 2023
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Otherwise I think this looks plausible, although I'd like to check with someone who knows this area of the codebase as obviously the price of getting this wrong is spectacular. Speaking of which, a unit test or something seems important here.

import { Container, WidgetLayoutStore } from "../../../stores/widgets/WidgetLayoutStore";
import { getConfigLivestreamUrl, startJitsiAudioLivestream } from "../../../Livestream";
import { WidgetPermissionCustomisations } from "../../../customisations/WidgetPermissions";
import { ElementWidget } from "../../../stores/widgets/StopGapWidget";
Copy link
Member

Choose a reason for hiding this comment

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

This class has a comment saying, // TODO: Don't use this because it's wrong, so I'm not not sure this is what we want. Could we not just pass the json object into the customisation function? Why do we need to construct a Widget object from it first?

Copy link
Member

Choose a reason for hiding this comment

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

(the comment is trying to say that "StopGap" is temporary, but that was years ago)

@dbkr
Copy link
Member

dbkr commented Jan 18, 2023

Having checked, it seems like this interface (customisation endpoints) has mostly been deprecated in favour of the modules system: https://github.com/matrix-org/matrix-react-sdk-module-api and https://github.com/vector-im/element-web-ilag-module for an example, so the intention would be to remove this API at some point. You may want to consider making this interface available via the modules system rather than have to do the migration in the future.

@maheichyk
Copy link
Contributor Author

Okay, thanks for feedback, then the plan is to try to use the module system.

@maheichyk
Copy link
Contributor Author

Closing this PR in favor of #10121

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

Labels

T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants