Bypass feature validation for dynamic tenant features #9523

jrafanie · 2025-07-22T20:13:16Z

Bypass feature validation for dynamic tenant features

We expect a one to one feature assignment with existing product features.
This doesn't work with dynamic tenant features.

This dynamic tenant feature was only ever implemented for tenant quotas.

Related to:
#5123
#5129
#5142

Fixes: #9512

Add a test for Settings, Access Control, Tenant Add and Delete

This recreates the issue found in #9512

Fryguy · 2025-07-23T14:12:51Z

As discussed

tactically, if we change the role_allows to use x_node it would eliminate the params code, and it should just work...long term, there's gotta be a better way that avoids override role_allows at all, and passing in the correct feature from the caller

role_allows shouldn't be dynamically be building features though - something else should be doing that - I'm not sure how the toolbar code is working though
what I think has to happen long term, is we shouldn't be using basic buttons, but should have custom subclasses of the button for tenant stuff

jrafanie · 2025-07-24T16:57:38Z

Closing in favor of #9527

jrafanie · 2025-08-13T18:25:54Z

cc @asirvadAbrahamVarghese I used some of your pre-canned functions for the cypress test - let me know if you have better options than what I used.

asirvadAbrahamVarghese · 2025-08-14T10:50:28Z

cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js

+    cy.toolbar(toolBarConfigMenu, 'Add child Tenant to this Tenant');
+    cy.get('input#name').type(initialTenantName);
+    cy.get('input#description').type(initialTenantDescription);
+    cy.contains('.bx--btn--primary', 'Add').click();


if this is a form can you try using getFormFooterButtonByType
cy.getFormFooterButtonByType('Add', 'submit') should work (not sure about the html button type here)

asirvadAbrahamVarghese · 2025-08-14T10:53:09Z

cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js

+    ]);
+
+    cy.toolbar(toolBarConfigMenu, 'Add child Tenant to this Tenant');
+    cy.get('input#name').type(initialTenantName);


Here we can use getFormInputFieldById
cy.getFormInputFieldById('name').type(initialTenantName); should be good.

asirvadAbrahamVarghese · 2025-08-14T10:54:38Z

cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js

+
+    cy.toolbar(toolBarConfigMenu, 'Add child Tenant to this Tenant');
+    cy.get('input#name').type(initialTenantName);
+    cy.get('input#description').type(initialTenantDescription);


similar to above: cy.getFormInputFieldById('description').type(initialTenantDescription);

asirvadAbrahamVarghese · 2025-08-14T11:01:10Z

I haven't come across many form elements yet, but using these commands consistently across tests will help us generalize the selectors in here 😃

asirvadAbrahamVarghese · 2025-08-14T11:29:57Z

cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js

+  // flash message assertions
+  flashMessageOperationAdded: 'added',
+  flashMessageOperationDeleted: 'delete',
+  flashTypeSuccess: 'success',


I wanted to use a constant for the flash message type to avoid repetition, but forgot to follow through: #9554

I'll do that here and update collect_logs as well

jrafanie · 2025-08-14T22:11:49Z

cypress/e2e/ui/Settings/Application-Settings/edit_collect_logs.cy.js

@@ -195,7 +192,7 @@ function saveButtonValidation() {
    .should('be.enabled')
    .click();
  // Validating confirmation flash message
-  cy.expect_flash(flashTypeSuccess, flashMessageSettingsSaved);
+  cy.expect_flash(flashClassMap.success, flashMessageSettingsSaved);


I changed this one too...

jrafanie · 2025-08-14T22:13:05Z

@asirvadAbrahamVarghese please take a look at the test changes... I also reduced the repetitive constant definition and assignment.

asirvadAbrahamVarghese · 2025-08-15T17:32:45Z

@jrafanie looks fine to me now, thanks!

jrafanie · 2025-08-18T16:44:53Z

cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js

+  // CRUD actions
+  const FLASH_MESSAGE_OPERATION_ADDED = 'added';
+  const FLASH_MESSAGE_OPERATION_DELETED = 'delete';
+  const DELETE_ITEM = 'Delete this item';


This list of constants can be updated/consolidated when we add more access control tests. It's hard to decide what should be constants vs. inline.

My thinking is:

If it's used in a selector and unique to the test, it probably should be left as an inline string to make the test more readable. If it's used often enough, it should should become a shared function.

Simple text we input for CRUD will likely be reused by other tests so constants make sense there

Shared navigation for the whole set of tests and not unique to the test could be constants...

I don't know, that's my thinking here.

jrafanie · 2025-08-18T16:49:26Z

@Fryguy can you review the ruby side here? This is the workaround for the per tenant feature thing.

We expect a one to one feature assignment with existing product features. This doesn't work with dynamic tenant features. This dynamic tenant feature was only ever implemented for tenant quotas. Related to: ManageIQ#5123 ManageIQ#5129 ManageIQ#5142 Fixes: ManageIQ#9512

This recreates the issue found in ManageIQ#9512

* Drop repetive constant definition and assignment * Generalize the names of constants * Capitalize constants as per conventions

jrafanie · 2025-08-19T23:55:36Z

Rebase/Updated after dropping the edit collect logs changes that were also in #9546, which was merged cc @asirvadAbrahamVarghese

Fryguy · 2025-08-21T16:07:33Z

Backported to spassky in commit 3118187.

commit 311818769ace2068244ff5b4f64d177e8c4a0909
Author: Jason Frey <[email protected]>
Date:   Wed Aug 20 15:32:48 2025 -0400

    Merge pull request #9523 from jrafanie/skip-product-feature-validation-for-tenant-dynamic-product-features
    
    Bypass feature validation for dynamic tenant features
    
    (cherry picked from commit 71881e11962ff4bb296b306b2be26857bbd326a6)

…n-for-tenant-dynamic-product-features Bypass feature validation for dynamic tenant features (cherry picked from commit 71881e1)

jrafanie requested a review from a team as a code owner July 22, 2025 20:13

jrafanie added the wip label Jul 22, 2025

Fryguy changed the title ~~WIP - bypass feature validation for dynamic tenant features~~ [WIP] bypass feature validation for dynamic tenant features Jul 22, 2025

jrafanie mentioned this pull request Jul 24, 2025

[WIP] Extract the tenant id from the x_node #9527

Closed

jrafanie closed this Jul 24, 2025

jrafanie reopened this Aug 13, 2025

jrafanie force-pushed the skip-product-feature-validation-for-tenant-dynamic-product-features branch from c8c8ee5 to c816791 Compare August 13, 2025 18:19

jrafanie added bug and removed wip labels Aug 13, 2025

jrafanie force-pushed the skip-product-feature-validation-for-tenant-dynamic-product-features branch from c816791 to 6484ca9 Compare August 13, 2025 18:23

jrafanie changed the title ~~[WIP] bypass feature validation for dynamic tenant features~~ Bypass feature validation for dynamic tenant features Aug 13, 2025

jrafanie assigned Fryguy Aug 13, 2025

asirvadAbrahamVarghese reviewed Aug 14, 2025

View reviewed changes

jrafanie force-pushed the skip-product-feature-validation-for-tenant-dynamic-product-features branch from 6484ca9 to f601874 Compare August 14, 2025 21:21

jrafanie commented Aug 14, 2025

View reviewed changes

jrafanie added the spassky/yes label Aug 15, 2025

jrafanie commented Aug 18, 2025

View reviewed changes

asirvadAbrahamVarghese mentioned this pull request Aug 19, 2025

Follow up works from #9516 #9546

Merged

jrafanie mentioned this pull request Aug 19, 2025

[WIP] - Added automated tests with cypress for Tenants form #9532

Open

jrafanie added 3 commits August 19, 2025 19:54

Add a test for Settings, Access Control, Tenant Add and Delete

8b7c094

This recreates the issue found in ManageIQ#9512

Reuse flashClassMap from just merged ManageIQ#9554

4b749f2

Simplify the defining of constants and access

6f11925

* Drop repetive constant definition and assignment * Generalize the names of constants * Capitalize constants as per conventions

jrafanie force-pushed the skip-product-feature-validation-for-tenant-dynamic-product-features branch from f601874 to 6f11925 Compare August 19, 2025 23:54

Fryguy approved these changes Aug 20, 2025

View reviewed changes

Fryguy merged commit 71881e1 into ManageIQ:master Aug 20, 2025
18 checks passed

Fryguy added a commit that referenced this pull request Aug 21, 2025

Merge pull request #9523 from jrafanie/skip-product-feature-validatio…

3118187

…n-for-tenant-dynamic-product-features Bypass feature validation for dynamic tenant features (cherry picked from commit 71881e1)

Fryguy added spassky/backported and removed spassky/yes labels Aug 21, 2025

Bypass feature validation for dynamic tenant features #9523

Bypass feature validation for dynamic tenant features #9523

Uh oh!

Conversation

jrafanie commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bypass feature validation for dynamic tenant features

Add a test for Settings, Access Control, Tenant Add and Delete

Uh oh!

Fryguy commented Jul 23, 2025

Uh oh!

jrafanie commented Jul 24, 2025

Uh oh!

jrafanie commented Aug 13, 2025

Uh oh!

asirvadAbrahamVarghese Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asirvadAbrahamVarghese Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asirvadAbrahamVarghese Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

asirvadAbrahamVarghese commented Aug 14, 2025

Uh oh!

asirvadAbrahamVarghese Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

jrafanie Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

jrafanie Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

jrafanie commented Aug 14, 2025

Uh oh!

asirvadAbrahamVarghese commented Aug 15, 2025

Uh oh!

jrafanie Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

jrafanie commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrafanie commented Aug 19, 2025

Uh oh!

Uh oh!

Fryguy commented Aug 21, 2025

Uh oh!

Uh oh!

jrafanie commented Jul 22, 2025 •

edited

Loading

asirvadAbrahamVarghese Aug 14, 2025 •

edited

Loading

asirvadAbrahamVarghese Aug 14, 2025 •

edited

Loading

jrafanie commented Aug 18, 2025 •

edited

Loading