Skip to content

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Apr 3, 2025

Part of #47412.

Create one endpoint to retrieve menu information. Two pieces of information are retrieved:

  • authorized_menu_items: the list of menu items the user has access to
  • extra_menu_items: the list of menu items provided by the auth manager. We used to have a endpoint to retrieve that information. I removed that endpoint since it is unnecessary to have 2 endpoints

I updated the front-end code to no longer use the /ui/auth/links endpoint but the new one. Once that merged, we need to update the front-end code to display/hide menu items depending on whether it belongs to authorized_menu_items. @bbovenzi


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Apr 3, 2025
@vincbeck vincbeck force-pushed the vincbeck/menu_authz branch from bf5cd0a to 8e5257f Compare April 3, 2025 19:40
@vincbeck vincbeck force-pushed the vincbeck/menu_authz branch 2 times, most recently from a5001be to a35b2fe Compare April 3, 2025 20:51
@vincbeck vincbeck requested a review from bbovenzi April 4, 2025 13:03
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Nice.

Yes I agree that this mapping MenuItem.EVENTS: RESOURCE_AUDIT_LOG is confusing. (Even in the UI event vs asset event, I find AUDIT_LOG less confusing)

A couple of small nits, but looking good to me.

@vincbeck vincbeck force-pushed the vincbeck/menu_authz branch from a35b2fe to 11c60b9 Compare April 7, 2025 13:46
@vincbeck
Copy link
Contributor Author

vincbeck commented Apr 7, 2025

Nice.

Yes I agree that this mapping MenuItem.EVENTS: RESOURCE_AUDIT_LOG is confusing. (Even in the UI event vs asset event, I find AUDIT_LOG less confusing)

A couple of small nits, but looking good to me.

I renamed "Events" to "Audit log" then

@vincbeck
Copy link
Contributor Author

vincbeck commented Apr 7, 2025

Tests are unrelated to this PR. Merging

@vincbeck vincbeck merged commit e187961 into apache:main Apr 7, 2025
93 of 95 checks passed
@vincbeck vincbeck deleted the vincbeck/menu_authz branch April 7, 2025 15:03
@dstandish
Copy link
Contributor

Was this the right change? Asset events are different from events (a.k.a. event log a.k.a. audit log) and also different from assets.

simonprydden pushed a commit to simonprydden/airflow that referenced this pull request Apr 8, 2025
@pierrejeambrun
Copy link
Member

Was this the right change? Asset events are different from events (a.k.a. event log a.k.a. audit log) and also different from assets.

There's no asset event page yet, but there is an 'AUDIT_LOG' one. We will most likely add the ASSET_EVENTS one later when the asset event page is implemented.

@dstandish
Copy link
Contributor

Was this the right change? Asset events are different from events (a.k.a. event log a.k.a. audit log) and also different from assets.

There's no asset event page yet, but there is an 'AUDIT_LOG' one. We will most likely add the ASSET_EVENTS one later when the asset event page is implemented.

i see, so it only looked like a rename in the diff but was not a rename; just, we did not need it yet.

@pierrejeambrun
Copy link
Member

I think so yes

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

Labels

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants