cal.com icon indicating copy to clipboard operation
cal.com copied to clipboard

Non premium usernames being shown as premium during sign up due to an issue in debounce logic

Open theonly1me opened this issue 1 year ago • 3 comments

Found a bug? Please fill out the sections below. 👍

Issue Summary

As per this logic from packages/lib/server/username.ts the premium usernames are either <=4 characters or are present in the username blacklist present the URL from process.env.USERNAME_BLACKLIST_URL. However, as you can see from the video below, the usernames that are longer than >=4 characters and not in the blacklist are still shown as premium and unavailable prompting users to upgrade.

https://github.com/calcom/cal.com/assets/19223383/e3ed651e-e100-4ce4-8a06-1da61aac1007

Steps to Reproduce

  1. Go to https://app.cal.com/getting-started
  2. Enter a username that is longer than 4 characters, fast - for example: liukang
  3. You'll see a premium username error even though liukang is >4 characters and isn't in the blacklist Screenshot 2024-06-30 at 7 50 21 PM
  4. Add a - at the end of luikang -> liukang-. As you can see from the image below, once you add - the premium name error goes away. Screenshot 2024-06-30 at 7 51 31 PM
  5. Now remove the - from the end and the name liukang will no longer show the premium validation error Screenshot 2024-06-30 at 7 53 07 PM
  6. You can click on update and save the name too Screenshot 2024-06-30 at 8 08 41 PM

Any other relevant information. For example, why do you consider this a bug and what did you expect to happen instead?

This is a bug because it's marking usernames that are not premium as being premium, additionally provides a janky user experience when trying to sign up to cal.com, which could negatively impact user perception.

Actual Results

  • usernames that aren't premium are marked as premium even though the POST /api/username responds with the following:
{
    "available": true,
    "premium": false,
    "suggestedUsername": ""
}

Expected Results

  • Only the usernames that are satisfy the criteria should be marked as premium
  • The UI should respect the API response from POST /api/username

Technical details

  • The reason for this is a bug in the debounce logic on the UI in PremiumTextField.tsx
  • The debounce doesn't apply and if you see the network tab on the video attached or in your browser, you'll be able to see that there are API calls to POST /api/username on every OnChange.
  • The main reason for this is because the inputUsernameValue prop changes on every onChange event on the username TextField (since it updates the external state via setInputUsernameValue)
  • The way to resolve this is by cleaning up the debouncedAPICall in the useEffect cleanup function when the component unmounts.
  • This bug is also present on the UsernameTextField.tsx

Additional feedback on these files:

  • The PremiumTextField.tsx and UsernameTextField.tsx practically do the same thing, except one also checks whether a username is Premium for cal.com cloud instances. These two components can be merged into a single component.
  • The useMemo hook is an unnecessary over optimization in this case and can be removed

Evidence

  • Please refer to the video & screenshots I've added above
  • I've tested it on cal.com production website

theonly1me avatar Jun 30 '24 14:06 theonly1me

Hey @theonly1me, I'm here to assist you with bug fixes, answer questions, and even guide you on becoming a contributor. Let's squash those bugs together! 🤖

To address the issue with non-premium usernames being incorrectly marked as premium during the sign-up process, you need to fix the debounce logic in PremiumTextField.tsx and UsernameTextField.tsx. Here is the corrected debounce logic:

const debouncedApiCall = useMemo(
  () =>
    debounce(async (username: string) => {
      const { data } = await fetchUsername(username, null);
      setMarkAsError(!data.available && !!currentUsername && username !== currentUsername);
      setIsInputUsernamePremium(data.premium);
      setUsernameIsAvailable(data.available);
    }, 150),
  [currentUsername]
);

useEffect(() => {
  if (!inputUsernameValue) {
    debouncedApiCall.cancel();
    return;
  }
  debouncedApiCall(inputUsernameValue);
}, [debouncedApiCall, inputUsernameValue]);

useEffect(() => {
  return () => {
    debouncedApiCall.cancel();
  };
}, [debouncedApiCall]);

This logic ensures that the debouncedApiCall function is correctly debounced and that it sets the isInputUsernamePremium state based on the response from the fetchUsername API call. This will ensure that only premium usernames are marked as such during the sign-up process [1].

Additionally, the fetchUsername function implementation is as follows:

type ResponseUsernameApi = {
  available: boolean;
  premium: boolean;
  message?: string;
  suggestion?: string;
};

export async function fetchUsername(username: string, orgSlug: string | null) {
  const response = await fetch("/api/username", {
    credentials: "include",
    method: "POST",
    body: JSON.stringify({
      username: username.trim(),
      orgSlug: orgSlug ?? undefined,
    }),
    headers: {
      "Content-Type": "application/json",
    },
  });
  const data = (await response.json()) as ResponseUsernameApi;
  return { response, data };
}

This function sends a POST request to the /api/username endpoint and handles the API response by parsing it as JSON [2].

By implementing these changes, you should be able to resolve the issue of non-premium usernames being incorrectly marked as premium.

To continue talking to Dosu, mention @dosu.

dosubot[bot] avatar Jun 30 '24 14:06 dosubot[bot]

Please review #15629, it fixes this issue. Also, would be great if this can be assigned to me, thanks.

Video demo of the fix is present in the PR description.

theonly1me avatar Jun 30 '24 14:06 theonly1me

@keithwillcode, this one's a bit out of my wheelhouse, could you step in and provide your guidance?

To continue talking to Dosu, mention @dosu.

dosubot[bot] avatar Jun 30 '24 14:06 dosubot[bot]