fix(classification): Apps to Integrations name change #3726
Conversation
|
|
greg-in-a-box
changed the title
greg-in-a-box
changed the title
n31
force-pushed
the
display-apps-as-integrations
branch
from
d505d10
to
811fc18
Compare
n31
force-pushed
the
display-apps-as-integrations
branch
from
811fc18
to
fdce2c6
Compare
| test('should render SecurityControls with Integration label when using SHORT controlsFormat and shouldDisplayAppsAsIntegrations is true', () => { | ||
| wrapper.setProps({ controlsFormat: SHORT, shouldDisplayAppsAsIntegrations: true }); | ||
| expect(wrapper.find('SecurityControlsItem').prop('message').id).toBe( | ||
| messages.shortSharingDownloadIntegration.id, |
There was a problem hiding this comment.
we should be asserting against the hardcoded string, not the id or the variable, if the message changes, none of the new tests will fail if someone else accidentally changes it. this goes for all the tests you are asserting below checking for the correct message to appear.
also this component small enough to convert the test file to RTL where you could see the literal string in the dom so that its not mocked in the rest of the tests.
| 'should include correct message when integration download is restricted by %s and integrations are less than maxAppCount and shouldDisplayAppsAsIntegrations is true', | ||
| listType => { | ||
| accessPolicy = { | ||
| app: { | ||
| accessLevel: listType, | ||
| apps: [{ displayText: 'a' }, { displayText: 'b' }, { displayText: 'c' }], | ||
| }, | ||
| }; | ||
| const expectedMessage = integrationRestrictionsMessageMap[listType][WITH_APP_LIST]; | ||
|
|
||
| expect(expectedMessage).toBeTruthy(); | ||
| expect(getFullSecurityControlsMessages(accessPolicy, 3, true)).toEqual([ | ||
| { | ||
| message: { | ||
| ...expectedMessage, | ||
| values: { appNames: 'a, b, c' }, | ||
| }, | ||
| }, | ||
| ]); | ||
| }, | ||
| ); | ||
|
|
There was a problem hiding this comment.
I think we need assert against the hardcoded message also, the test is essentially check if the input is provide the correct messages so using the variable here might not be useful because someone could still change the message and these unit tests will still pass same with the test below.
if we do not care about the message in these unit tests and we are just testing the function itself, then maybe updating the test name to reflect that
Replaced 'application' references in Classification modal with 'integration', based on optional flag variable (by default set to false, so I won't break existing code and will display 'application' as usual). When flag is passed as 'True' then 'Integrations' will be displayed, otherwise 'Applications'. Also, unit tests are added for related changes.