Problem/Motivation

  • In k8s_query_k8s_entity_views_access_with_namespace_alter() function, add the remaining K8s routes to $target_route_names array
CommentFileSizeAuthor
#15 3221336-3x-15.patch14.3 KBbaldwinlouie

Issue fork cloud-3221336

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

baldwinlouie created an issue. See original summary.

baldwinlouie’s picture

Status: Active » Needs review

@yas and @xiaohuaguan Please review this patch that modifies the k8s_query_k8s_entity_views_access_alter and k8s_query_k8s_entity_views_access_with_namespace_alter functions.

The patch does the following:
1. Merges the two k8s_query_k8s_entity_views_access_alter and k8s_query_k8s_entity_views_access_with_namespace_alter functions into one since ALL K8s entities need to check for any/own when building a listing page.
2. Refactored the K8s*ViewsData. I moved the access query tag into the K8sViewsDataBase parent class, since all entities will now use it.

yas’s picture

Issue summary: View changes

@baldwinlouie

Thank you for the patch taking care of the K8s resource permissions. It looks good to me.

@xiaohua-guan

Could you please review the patch, too? What do you think?

xiaohua guan’s picture

@baldwinlouie

Thanks for your patch. It looks good to me except that the code below in K8sNamespaceViewsData can be removed, I think.

    $data['k8s_namespace']['table']['base']['access query tag'] = 'k8s_entity_views_access';
baldwinlouie’s picture

@xiaohuaguan, Thank you for the review. I've updated the PR with your feedback.

yas’s picture

@xiaohua-guan

Please let us gives your thoughts as a final confirmation before we merge the patch.

Thanks

xiaohua guan’s picture

@baldwinlouie @yas

Thanks for your new code. It looks good to me. Maybe in other ticket, we can remove $namespaced_routes = [] by checking whether there is namespace field in the entity type.

yas’s picture

Status: Needs review » Reviewed & tested by the community

@xiaohua-guan

Thank you for you review and suggestion. I'll merge the patch to 4.x and close this issue as Fixed.

Maybe in other ticket, we can remove $namespaced_routes = [] by checking whether there is namespace field in the entity type.

Yes, we should address that in another issue.

Thanks

  • yas committed 9a7927d on 4.x authored by baldwinlouie
    Issue #3221336 by baldwinlouie, yas, Xiaohua Guan: Add remaining k8s...

yas’s picture

Status: Reviewed & tested by the community » Fixed
yas’s picture

Status: Fixed » Needs work

@baldwinlouie

I re-opened this issue due to a coding standard violation: https://www.drupal.org/pift-ci-job/2109620. Sorry but I didn't notice it.

Could you please create a patch for 3.x and a hotfix for 4.x?

Thanks

baldwinlouie’s picture

StatusFileSize
new14.3 KB

@yas, attaching patch for 3.x branch as well.

baldwinlouie’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: 3221336-3x-15.patch, failed testing. View results

yas’s picture

Status: Needs work » Reviewed & tested by the community

@baldwinlouie

Thank you for the updates. I'll merge the patch to 3.x and 4.x and close this issue as Fixed.

  • yas committed 35812ed on 3.x authored by baldwinlouie
    Issue #3221336 by baldwinlouie, yas, Xiaohua Guan: Add remaining k8s...

  • yas committed aaca5d4 on 4.x authored by baldwinlouie
    Issue #3221336 by baldwinlouie, yas, Xiaohua Guan: Hotfix - Add...
yas’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.