feat: Add Hours, Minutes, Seconds fields while taking input as idleTimeLimit

[thepiyush-303]

Proposed changes (including videos or screenshots)

Before

After

Issue(s)

#33566

[dionisio-bot[bot]]

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[bot]]

🦋 Changeset detected

Latest commit: b653d232452fcee88750c51b07c0ac1fe0bd499b

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/models	Patch
@rocket.chat/network-broker	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

[thepiyush-303]

Hello @dougfabris , Can you please review my changes.

[Gustrb]

Hey @thepiyush-303 thanks for your contribution :) There is no need to ping anyone, your PR will be reviewed as soon as possible.

But taking a closer look at it, it concerns me that we would need to have 3 redundant settings, I think that if we want to follow the approach of having 3 input fields, we could convert their respective values to seconds and just store that, wdyt?

[thepiyush-303]

I understand your concern about having redundant settings. Converting the input values to seconds and storing that sounds like a more efficient approach.

However, I’ve encountered a couple of issues while implementing this. Firstly, when I change the field values and save them, they revert to the default values when I navigate back to edit them again.

Additionally, I'm running into an "invalid params" error when I attempt to save the values. This appears to be due to the extra fields not being recognized by the form.

[Gustrb]

I am not a frontend dev, but I don't think there is a need to store the value the same way the user put in, for example:

the user inserts 61 minutes, we convert it to 3660 seconds and whenever we load the page we display the value as 1 hour and 1 minute.

it does not sound like a complicated algorithm

The invalid fields error is probably because the backend is not expecting those new fields you are sending

[thepiyush-303]

I am not a frontend dev, but I don't think there is a need to store the value the same way the user put in, for example:

the user inserts 61 minutes, we convert it to 3660 seconds and whenever we load the page we display the value as 1 hour and 1 minute.

it does not sound like a complicated algorithm

The invalid fields error is probably because the backend is not expecting those new fields you are sending

Thanks for your input! The issue is that the user needs to provide input in hours, minutes, and seconds. Once they enter the values, if they want to see the changes in the fields, the backend would indeed need to support that. That’s why I chose the approach in the PR. If I'm misunderstanding something, please let me know!

[Gustrb]

That is what I mean, the backend should not support multiple settings because we should not have multiple settings. I am pretty sure no one will approve that, the settings for hour, minute and second should be just comestic, internally we should convert to seconds, otherwise we will need to have a specifc way to fetch a specific user preference everytime, we should avoid that bacause it is error prone and confusing

[thepiyush-303]

Got it, thanks for the clarification! Should I leave this issue as is, or is there anything else you'd like me to address?

[Gustrb]

I mean, if you have the time I think it would be a cool addition to do it in the client

[thepiyush-303]

I mean, if you have the time I think it would be a cool addition to do it in the client

but since you mentioned that having multiple settings might not be approved by other collaborators, I'm hesitant to continue with this approach. If it’s a significant enhancement, I’d be happy to explore it further if it aligns with the team's direction.

[Gustrb]

What I mean by that is, we have user preferences that we store in the database, I don't think those inputs should be mapped to actual values in the database, I think it would be interesting to convert their values to seconds and keep storing seconds. The only change that I think would fit in this case is adding the inputs so we can have a better granularity in that field, I don't think that granularity should go all the way to the databse, it should be nothing more than an abstraction.

[thepiyush-303]

@Gustrb I have made the requested changes.

[Gustrb]

Hey @thepiyush-303 did you update the PR? I couldn't see your changes :)

also, thanks for your dedication

[thepiyush-303]

Hello @Gustrb , I have made the changes you suggested. Specifically:

1. I updated my approach to storing values in three different settings, aligning with your earlier suggestion.
2. I have tested the changes, and everything is working fine.

Additionally, I have added a new file. Could you please advise on the appropriate location for this file?

Finally, could you suggest any changes needed to ensure that my code adheres to Rocket's coding standards?

[thepiyush-303]

How can i solve my deploy-preview check? if i make any mistake while setting up the project locally?

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.36%. Comparing base (4460503) to head (110689c). Report is 2 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #33708   +/-   ##
========================================
  Coverage    59.36%   59.36%           
========================================
  Files         2819     2819           
  Lines        67652    67652           
  Branches     15022    15022           
========================================
  Hits         40159    40159           
  Misses       24678    24678           
  Partials      2815     2815           

[thepiyush-303]

closing this because of bad commits. will create a new PR