[HOLD for payment 2023-12-28] [HOLD for payment 2023-12-12] Room - Inconsistent error message when entering [[<<> in the room name field

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.7-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team_
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to + > Start chat > Room.
3. Click Create room without entering room name.
4. Type [[<<> (don't copy paste) in the name field.

Expected Result:

The error message for invalid room name should be consistent.

Actual Result:

The error message for invalid room name is not consistent.
The error message changes from "Room names can only include lowercase letters, numbers and hyphens" to "Invalid characters" when typing [[<<>.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6298720_1701526862025.bandicam_2023-12-02_20-18-13-110.mp4

View all open jobs on GitHub

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @grgia (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[kbecciv]

Not reproduced on Production

bandicam.2023-12-02.20-21-55-204.mp4

[shubham1206agra]

Offending PR #30996
But I think the implementation of PR is correct
This is a problem with the validation of Form and FormProvider

[ishpaul777]

Proposal

Problem

Room - Inconsistent error message when entering [[<<> in the room name field

Root Cause

We have validation implemented for input value to be not html tags, the error meesage if inputValue is matched to html regex here and it supercede any other error message

Changes Suggested

Have a prop skipValidateForHtmlTags accepts the array of input id's for which it skips validation for tags and pass from WorkspaceNewRoomPage and RoomNamePage as we have the validation for room name already, even if we use same logic Form component and ignore the WHITELISTED_TAGS for the case inconsistent error message still shows up, when <<<<tesxt>so i think we can skip this validation as it introduce a inconsistency and not strictly required,

Alternatively we can consider have a more descriptive error message something like "Room names cannot include Html Tags"

[bernhardoj]

My bad, I was wrong with my comment here.

Proposal

Please re-state the problem that we are trying to solve in this issue.

After inputting <<>, the error message changes to "Invalid character".

What is the root cause of that problem?

In FormProvider, we have a validation to check if there is any HTML tag and we will show "Invalid character" error message which will supercede any error message from the component validation.

App/src/components/Form/FormProvider.js

Lines 131 to 159 in 1682be4

	_.each(trimmedStringValues, (inputValue, inputID) => {
	// If the input value is empty OR is non-string, we don't need to validate it for HTML tags
	if (!inputValue || !_.isString(inputValue)) {
	return;
	}
	const foundHtmlTagIndex = inputValue.search(CONST.VALIDATE_FOR_HTML_TAG_REGEX);
	const leadingSpaceIndex = inputValue.search(CONST.VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX);
	// Return early if there are no HTML characters
	if (leadingSpaceIndex === -1 && foundHtmlTagIndex === -1) {
	return;
	}
	const matchedHtmlTags = inputValue.match(CONST.VALIDATE_FOR_HTML_TAG_REGEX);
	let isMatch = _.some(CONST.WHITELISTED_TAGS, (r) => r.test(inputValue));
	// Check for any matches that the original regex (foundHtmlTagIndex) matched
	if (matchedHtmlTags) {
	// Check if any matched inputs does not match in WHITELISTED_TAGS list and return early if needed.
	for (let i = 0; i < matchedHtmlTags.length; i++) {
	const htmlTag = matchedHtmlTags[i];
	isMatch = _.some(CONST.WHITELISTED_TAGS, (r) => r.test(htmlTag));
	if (!isMatch) {
	break;
	}
	}
	}
	// Add a validation error here because it is a string value that contains HTML characters
	validateErrors[inputID] = 'common.error.invalidCharacter';
	});

This happens after we migrate it from Form to FormProvider. In the HTML tag validation logic, we actually have a whitelist for some of the tags.

App/src/components/Form/FormProvider.js

Lines 148 to 155 in 1682be4

	// Check if any matched inputs does not match in WHITELISTED_TAGS list and return early if needed.
	for (let i = 0; i < matchedHtmlTags.length; i++) {
	const htmlTag = matchedHtmlTags[i];
	isMatch = _.some(CONST.WHITELISTED_TAGS, (r) => r.test(htmlTag));
	if (!isMatch) {
	break;
	}
	}

App/src/CONST.ts

Line 826 in 1682be4

WHITELISTED_TAGS: [/<>/, /< >/, /<->/, /<-->/, /<br>/, /<br\/>/],

If the tag is in the whitelist, we won't show the "Invalid character" error. This works correctly in Form because we return early if it's found in the whitelist.

App/src/components/Form.js

Lines 171 to 176 in 1682be4

	if (isMatch && leadingSpaceIndex === -1) {
	return;
	}
	// Add a validation error here because it is a string value that contains HTML characters
	validationErrors[inputID] = 'common.error.invalidCharacter';

One of the whitelist is <>. In our case, we input <<> and <> is the substring and found in the whitelist, so we shouldn't show the "Invalid character" error just like in prod.

this is how the validation works
match any HTML tag in the input (regex from BE) -> loop over the found/matched tags -> if one of time is not in the whitelist (whitelist regex), returns error

What changes do you think we should make in order to solve the problem?

Copy the early return logic from Form to FormProvider by early return if the tag is in the whitelist.

if (isMatch && leadingSpaceIndex === -1) {
    return;
}

This will not show the "Invalid character" error message if we input <<>.

isMatch is honestly a bit confusing to understand, I think we can rename it to isWhitelisted

Another issue:
Actually BE can't accept <<> (detected as HTML tag), but it can accept <>> (not detected as HTML tag, the regex is copied from the BE, that's why BE can accept this). It's the problem with our whitelist regex where /<>/ regex matches <<> substring. To fix it, we can update the regex to

/^<>$/

Or remove <> from the whitelist regex because <> is never detected as an HTML tag, so there is no point putting <> in the whitelist regex (preferable).

Fixing this will show "Invalid character" when we input <<>, but not when we input <>>.

[luacmartins]

We're reverting the offending PR.

[kowczarz]

There were some changes in form validation in old Form component during the migration and not all changes were included in the new FormProvider. It is fixed in this commit b9fc2f4

Here are the missing parts:

App/src/components/Form.js

Line 133 in 524593d

const trimmedStringValues = ValidationUtils.prepareValues(values);

App/src/components/Form.js

Lines 171 to 173 in 524593d

	if (isMatch && leadingSpaceIndex === -1) {
	return;
	}

[yuwenmemon]

Revert deployed so closing this out!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.7-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Revert "[Form Provider Refactor] RoomNameInput fixes" #32440

If no regressions arise, payment will be issued on 2023-12-12. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [Form Provider Refactor] RoomNameInput fixes #32432

If no regressions arise, payment will be issued on 2023-12-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR