fix(user-profile): enable saving empty bio to clear user profile field #33927

Curious-Goblin · 2024-11-09T16:37:33Z

Proposed changes

This pull request enables the ability to save an empty bio in the user profile to clear out the bio field, which was previously uneditable if left empty.

This fix addresses an issue where users could not save changes if they attempted to clear their bio.
Implements validation adjustments to allow empty strings in the bio field.

Issue(s)

Fixes Cannot Set Bio to Empty #33783

Steps to test or reproduce

Navigate to the user profile section in the Rocket.Chat application.
Attempt to clear the bio field and save changes.
Confirm that the profile successfully updates with an empty bio field.

Checklist

I have read the Contributing Guide.
I have signed the CLA.
Lint and unit tests pass locally.
Added tests to verify this fix.
Updated documentation as needed.

dionisio-bot · 2024-11-09T16:37:36Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is missing the 'stat: QA assured' label
This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2024-11-09T16:37:38Z

🦋 Changeset detected

Latest commit: 0ad5656

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 35 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch
@rocket.chat/uikit-playground	Patch
@rocket.chat/api-client	Patch
@rocket.chat/apps	Patch
@rocket.chat/core-services	Patch
@rocket.chat/cron	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/freeswitch	Patch
@rocket.chat/fuselage-ui-kit	Patch
@rocket.chat/gazzodown	Patch
@rocket.chat/livechat	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/ui-contexts	Patch
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/license	Patch
@rocket.chat/omnichannel-services	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/network-broker	Patch
@rocket.chat/models	Patch
@rocket.chat/ui-avatar	Patch
@rocket.chat/ui-client	Patch
@rocket.chat/ui-video-conf	Patch
@rocket.chat/ui-voip	Patch
@rocket.chat/web-ui-registration	Patch
@rocket.chat/instance-status	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

.changeset/slimy-lamps-learn.md

apps/meteor/server/methods/saveUserProfile.ts

…e handling - Added a test case to ensure the bio field is correctly set to an empty string. - Updated user identity handling to support clearing the bio field.

apps/meteor/app/lib/server/functions/getAvatarSuggestionForUser.ts

apps/meteor/server/methods/saveUserProfile.ts

debdutdeb

there should be api test for this.

Curious-Goblin · 2024-11-19T10:12:28Z

Hi @debdutdeb and @Gustrb,
I’ve addressed all requested changes and have added the necessary tests. Could you please review the updates when you have time? Let me know if there’s anything else needed. Thank you!

Gustrb · 2024-11-19T12:23:23Z

Hey, I agree with @debdutdeb we should have an API test for this as well

apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts

- Added a check in the saveUserIdentity test suite to confirm that the setBio function is invoked when a user sets their bio to an empty string. - Introduced a stubbed function for setBio to monitor its invocation during the test. - Ensures proper behavior and validation of bio updates, including empty values.

- Added a test to validate that the saveUserProfile API correctly handles setting the bio field to an empty string. - Included validation to ensure bio updates persist correctly when cleared. - Verified compliance with bio length constraints and ensured no errors occur for empty input.

Curious-Goblin · 2024-11-20T12:51:29Z

Hey, I agree with @debdutdeb we should have an API test for this as well

Hi @Gustrb ,

I’ve added API tests for setting an empty bio field in the user profile. These tests include:

Validating that the saveUserProfile method allows clearing the bio by setting it to an empty string (bio: ' ').
Ensuring compliance with the bio length constraints while verifying the behavior for an empty bio input.

To the best of my knowledge, these cover the requested scenarios. Could you please review and let me know if they align with the tests you asked about? If not, I’d appreciate it if you could clarify which specific API tests are referred to here so I can address them.

Thank you!

Gustrb · 2024-11-20T13:08:50Z

Hey @Curious-Goblin no need to keep merging develop into your branch, you're good.
Let's run the CI to see, but looks good to me

Curious-Goblin · 2024-11-20T13:10:57Z

Hey @Curious-Goblin no need to keep merging develop into your branch, you're good. Let's run the CI to see, but looks good to me

ok, I will take care of it next time

apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts

- Added a new test file to check the behavior when updating a user's bio to an empty field. - Altered .mocharc.js configuration to include the new test file for execution. - The update is still a work in progress and may need further adjustments.

Based on feedback, the saveUserProfile.spec.ts test file is not required. The path added to .mocharc.js for running this test has also been removed. These changes align with the confirmation that unit tests for saveUserProfile are unnecessary, as the current API tests are sufficient.

Curious-Goblin · 2024-12-03T06:39:05Z

Hi @Gustrb,

I hope you're doing well! It's been a while since my last commit. I wanted to ask if you've had a chance to discuss it with the QA team. If there are any errors or improvements needed, please let me know—I’d be happy to address them promptly.

Thanks for your time and guidance!

Curious-Goblin · 2024-12-16T07:07:54Z

Hi @Gustrb ,
I wanted to follow up on this PR since it's been over a week without a response. Please let me know if any further changes or clarifications are needed from my side to move this forward. I’m happy to address any feedback

casalsgh · 2024-12-16T17:46:05Z

Asking Gustavo to check. He had some other pressing items and had to delay this for a bit

Curious-Goblin · 2024-12-22T18:04:21Z

hello @Gustrb @casalsgh, its been another week, can someone please look into this pr and see if it is good to be merged or please help to get this pr a QA assured tag so that it can be merged or if is has some issues then let me know, I would be more than happy to contribute.

reetp · 2025-01-07T12:14:36Z

hello @Gustrb @casalsgh, its been another week, can someone please look into this pr and see if it is good to be merged or please help to get this pr a QA assured tag so that it can be merged or if is has some issues then let me know, I would be more than happy to contribute.

Please note.

Nothing around here happens fast. It can take MONTHS.

There have also been the Christmas holidays which for many is up to two weeks and has only just finished.

It is with the team. You just have to be extremely patient.

Gustrb · 2025-01-07T12:46:43Z

Hey, we don't merge PR's with failing tests, please fix that, then I will look into QA-ing

Curious-Goblin · 2025-01-07T12:51:49Z

regarding the failing tests thought that these tests which are failing I did not think that they were not the results on my changes but I will now look into my pr and try to change things accordingly so that the tests not fail

Gustrb · 2025-01-07T13:07:44Z

The UI tests are not, but there is one API test that is:
should reject invalid bio values (e.g., null):

Curious-Goblin · 2025-01-07T13:14:56Z

I want to ask that if empty bio and null bio are the same thing or not because this PR is solely made for one to save their bio as empty, and if the empty bio == null then the test which is rejecting null bio should fail in order to achieve the required feature

Gustrb · 2025-01-07T13:43:01Z

apps/meteor/server/methods/saveUserProfile.ts

@@ -79,7 +79,7 @@ async function saveUserProfile(
 		await setUserStatusMethod(this.userId, settings.statusType as UserStatus, undefined);
 	}

-	if (user && settings.bio) {
+	if (user && settings.bio !== undefined) {


I guess that if you change this to != it will work. let javascript coerce null to undefined
(empty string does not coerce to undefined)

Ok, I will apply these changes

can you please re run the workflows so that we can see the tests are still failing or not ??

can you please give the approval to run the workflows ??

please run the tests once again to see whether the tests are failing of not

Curious-Goblin · 2025-01-09T12:06:39Z

The UI tests are not, but there is one API test that is: should reject invalid bio values (e.g., null):

I think that now this error should be resolved now, can you check on it please

Gustrb · 2025-01-09T12:35:14Z

the api tests are still failing

Curious-Goblin · 2025-01-10T17:19:29Z

The UI tests are not, but there is one API test that is: should reject invalid bio values (e.g., null):

how can I see this message in the workflows I am not able find it out, can you help me ?

and I have tried running the test locally but for some reason I am not able to up the temporary mongo database due to which i am not able to run e2e tests and therefore I am asking you to give the approval for running the workflows

Gustrb · 2025-01-10T17:40:25Z

to run the api tests (the ones that are failing you can do the following:

Open two terminals, I'll call them A and B
On terminal A: navigate to the RC folder and run: TEST_MODE=true yarn dsv
On terminal B: navigate to the RC folder and run yarn testapi

Note: only run the tests when the workspace is up and running

Curious-Goblin · 2025-01-11T11:33:32Z

to run the api tests (the ones that are failing you can do the following:
* Open two terminals, I'll call them A and B

* On terminal A: navigate to the RC folder and run: `TEST_MODE=true yarn dsv`

* On terminal B: navigate to the RC folder and run `yarn testapi`
Note: only run the tests when the workspace is up and running

Thank you very much for this guidance, I have now checked that the test which you told about, is passing successfully. Actually the problem was in the test, it was sending request to wrong api endpoint, upon fixing that the test passed without changing anything the saveUserProfile file.

Please give the approval to the workflows and see if anything more is required to be worked on.

Curious-Goblin · 2025-01-11T14:09:32Z

can you please give the approval to run the workflows ??

reetp · 2025-01-11T14:41:04Z

can you please give the approval to run the workflows ??

This workflow requires approval from a maintainer. <<<<<<<<<<<<<<

Please read the link above:
Learn more about approving workflows.

Approving workflow runs from public forks
When an outside contributor submits a pull request to a public repository, a maintainer with write access may need to approve some workflow runs.

The answer will be no because only rocket devs have permissions to do this. Please stop asking. Thanks.

Curious-Goblin · 2025-01-11T14:46:17Z

can you please give the approval to run the workflows ??

This workflow requires approval from a maintainer. <<<<<<<<<<<<<<

Please read the link above: Learn more about approving workflows.

Approving workflow runs from public forks
When an outside contributor submits a pull request to a public repository, a maintainer with write access may need to approve some workflow runs.

The answer will be no because only rocket devs have permissions to do this. Please stop asking. Thanks.

Gustrb has given the approvals for workflows for a several times and therefore I was asking to him so that we can move forward with the PR

reetp · 2025-01-11T15:00:02Z

Gustrb has given the approvals for workflows for a several times and therefore I was asking to him so that we can move forward with the PR

Ok. In which case you may need to be patient - many of the main devs do not work on the weekend.

Curious-Goblin · 2025-01-11T15:13:09Z

ok

Curious-Goblin · 2025-01-13T12:25:22Z

to run the api tests (the ones that are failing you can do the following:
* Open two terminals, I'll call them A and B

* On terminal A: navigate to the RC folder and run: `TEST_MODE=true yarn dsv`

* On terminal B: navigate to the RC folder and run `yarn testapi`
Note: only run the tests when the workspace is up and running
Thank you very much for this guidance, I have now checked that the test which you told about, is passing successfully. Actually the problem was in the test, it was sending request to wrong api endpoint, upon fixing that the test passed without changing anything the saveUserProfile file.

Please give the approval to the workflows and see if anything more is required to be worked on.

Hello gustrb, I request you to give the approval for running the workflows as on my system the test passed

Gustrb · 2025-01-13T12:33:14Z

Please, remove the changes in yarn.lock

Curious-Goblin · 2025-01-13T13:10:58Z

I have deleted the changes in the file.

But I made those changes in order to run the test which was written by me, I ran different commands written in the script but got that dependencies are missing like the one in the ss

To overcome I installed required some dependencies locally and globally and then I reached this command
TS_NODE_COMPILER_OPTIONS='{"module": "commonjs"}' yarn mocha tests/end-to-end/api/users.ts --config .mocharc.api.js which successfully ran my test but now as I reverted the changes to original this shows this error

Maybe I am wrong, I don't know the proper way to run my test.

I was doing as you directed but got some errors

Curious-Goblin · 2025-01-13T14:29:51Z

Please, remove the changes in yarn.lock

I have reverted the changes in yarn.lock can you now see it

fix(user-profile): enable saving empty bio to clear user profile field

fb10f64

Curious-Goblin requested a review from a team as a code owner November 9, 2024 16:37

Add changeset for user profile bio update

86cb4dd

Gustrb requested changes Nov 10, 2024

View reviewed changes

.changeset/slimy-lamps-learn.md Outdated Show resolved Hide resolved

apps/meteor/server/methods/saveUserProfile.ts Show resolved Hide resolved

feat(users): add test for setting empty bio field and patch bio updat…

bf964f3

…e handling - Added a test case to ensure the bio field is correctly set to an empty string. - Updated user identity handling to support clearing the bio field.

debdutdeb requested changes Nov 11, 2024

View reviewed changes

apps/meteor/app/lib/server/functions/getAvatarSuggestionForUser.ts Outdated Show resolved Hide resolved

apps/meteor/server/methods/saveUserProfile.ts Show resolved Hide resolved

debdutdeb requested changes Nov 11, 2024

View reviewed changes

chore: removed unrelated changes from previous commit

0881d36

Curious-Goblin requested review from debdutdeb and Gustrb November 13, 2024 08:07

Merge branch 'develop' into bio-empty

f3607b3

Merge branch 'develop' into bio-empty

1438805

Gustrb reviewed Nov 19, 2024

View reviewed changes

apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts Outdated Show resolved Hide resolved

Curious-Goblin force-pushed the bio-empty branch from 5f9ce92 to f3f9c69 Compare November 19, 2024 13:39

Curious-Goblin and others added 3 commits November 19, 2024 19:14

Merge branch 'develop' into bio-empty

5f9b1fa

Merge branch 'develop' into bio-empty

0c4fcec

Gustrb reviewed Nov 20, 2024

View reviewed changes

apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts Outdated Show resolved Hide resolved

casalsgh requested a review from Gustrb November 26, 2024 14:05

Curious-Goblin mentioned this pull request Dec 16, 2024

Cannot Set Bio to Empty #33783

Open

Gustrb reviewed Jan 7, 2025

View reviewed changes

feat: added check for invalid bio values

b1d17d2

fix: adjust error message and operator to address failing tests

f6ccc36

Curious-Goblin force-pushed the bio-empty branch from 9b8785e to f6ccc36 Compare January 10, 2025 17:26

fix(tests): fix the failing api test

f562cd8

fix: remove unintended changes in yarn.lock

0ad5656

fix(user-profile): enable saving empty bio to clear user profile field #33927

Are you sure you want to change the base?

fix(user-profile): enable saving empty bio to clear user profile field #33927

Conversation

Curious-Goblin commented Nov 9, 2024 • edited Loading

Proposed changes

Issue(s)

Steps to test or reproduce

Checklist

dionisio-bot bot commented Nov 9, 2024

changeset-bot bot commented Nov 9, 2024 • edited Loading

🦋 Changeset detected

debdutdeb left a comment

Choose a reason for hiding this comment

Curious-Goblin commented Nov 19, 2024

Gustrb commented Nov 19, 2024

Curious-Goblin commented Nov 20, 2024

Gustrb commented Nov 20, 2024

Curious-Goblin commented Nov 20, 2024

Curious-Goblin commented Dec 3, 2024

Curious-Goblin commented Dec 16, 2024

casalsgh commented Dec 16, 2024

Curious-Goblin commented Dec 22, 2024

reetp commented Jan 7, 2025

Gustrb commented Jan 7, 2025

Curious-Goblin commented Jan 7, 2025

Gustrb commented Jan 7, 2025

Curious-Goblin commented Jan 7, 2025

Gustrb Jan 7, 2025

Choose a reason for hiding this comment

Curious-Goblin Jan 7, 2025

Choose a reason for hiding this comment

Curious-Goblin Jan 9, 2025

Choose a reason for hiding this comment

Curious-Goblin Jan 9, 2025

Choose a reason for hiding this comment

Curious-Goblin Jan 10, 2025

Choose a reason for hiding this comment

Curious-Goblin commented Jan 9, 2025

Gustrb commented Jan 9, 2025

Curious-Goblin commented Jan 10, 2025 • edited Loading

Gustrb commented Jan 10, 2025

Curious-Goblin commented Jan 11, 2025

Curious-Goblin commented Jan 11, 2025

reetp commented Jan 11, 2025

Curious-Goblin commented Jan 11, 2025

reetp commented Jan 11, 2025

Curious-Goblin commented Jan 11, 2025

Curious-Goblin commented Jan 13, 2025

Gustrb commented Jan 13, 2025

Curious-Goblin commented Jan 13, 2025

Curious-Goblin commented Jan 13, 2025

Curious-Goblin commented Nov 9, 2024 •

edited

Loading

changeset-bot bot commented Nov 9, 2024 •

edited

Loading

Curious-Goblin commented Jan 10, 2025 •

edited

Loading