amaannawab923 commented on code in PR #45522:
URL: https://github.com/apache/superset/pull/45522#discussion_r4401953205
##########
superset/extensions/utils.py:
##########
@@ -240,14 +240,35 @@ def get_loaded_extension(
def build_extension_data(extension: LoadedExtension) -> dict[str, Any]:
+ from superset.extensions.view import permitted_page_paths,
registered_page_paths
+
manifest = extension.manifest
+ registered = set(registered_page_paths(current_app, manifest.id))
+ permitted = permitted_page_paths(current_app, manifest)
extension_data: dict[str, Any] = {
"id": manifest.id,
"publisher": manifest.publisher,
"name": extension.name,
"version": extension.version,
"description": manifest.description or "",
"dependencies": manifest.dependencies,
+ "contributions": {
+ **manifest.contributions.model_dump(exclude_none=True),
+ # Every declared page is reported, with what became of it.
+ # Dropping the ones that did not get a rule would make a page the
+ # server refused indistinguishable from one never declared, and
+ # the extension's own bundle calls registerPage for every page it
+ # ships -- so the frontend needs to tell "refused, nothing you can
+ # do" apart from "you did not declare this".
+ "pages": [
Review Comment:
Added in 26c667d, with the three cases exactly as you framed them: an open
page, a permission-gated page the user does not hold the permission for, and a
path a host route already serves, asserted as the `(registered, permitted)`
pairs `(true, true)`, `(true, false)` and `(false, false)`.
It calls the real serializer against a real Flask app with the colliding
route registered first, so the refused page is refused by the actual
registration path rather than by a stub.
Your test for whether it was worth having holds up: swapping the two flags
in `build_extension_data` fails that one test and nothing else, out of the 329
in `tests/unit_tests/extensions`.
Also added a second case asserting every declared page survives into the
payload, including the refused one. That is the property the comment above the
list claims -- a refused page has to stay distinguishable from one never
declared -- and it was likewise resting on nothing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
