Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow empty notes and comments #2525

Merged
merged 3 commits into from
Jun 4, 2022
Merged

Conversation

Ashesh3
Copy link
Member

@Ashesh3 Ashesh3 commented May 25, 2022

Fixes #2507

Patient Notes:
image

Resource details page:
image

@Ashesh3 Ashesh3 requested a review from a team May 25, 2022 21:56
@patelaryan7751
Copy link
Contributor

patelaryan7751 commented May 26, 2022

Hey @Ashesh3 I found a bug in the shifting page comment posting algorithm we can easily overide the check "Comment should contain 1 character" by giving space . You can view it here #2532 for more detail demonstration.

I think we need to also have a check for spaces it might go like this:

// Explanation
const notes= " Hello Doctor  "
console.log(notes,notes.length);
console.log(notes.trim(),notes.trim().length);

/* Output
Hello Doctor   15
Hello Doctor 12
*/

// Implementation
if (noteField.trim().length < 1) {
      Notification.Error({
        msg: "Note Should Contain At Least 1 Character",
      });
      return;
    }

Copy link
Contributor

@developedBySJ developedBySJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make changes suggested by @patelaryan7751

@Ashesh3
Copy link
Member Author

Ashesh3 commented May 26, 2022

Using /\S+/ to check if the string is empty or not.

image

@patelaryan7751
Copy link
Contributor

Fixes #2532

@nihal467
Copy link
Member

@Ashesh3 @gigincg is it good for testing ?

@nihal467 nihal467 added question Further information is requested and removed needs testing labels May 27, 2022
@Ashesh3
Copy link
Member Author

Ashesh3 commented May 27, 2022

@Ashesh3 @gigincg is it good for testing ?

Yep, it is!

@Ashesh3 Ashesh3 added needs testing and removed question Further information is requested labels May 27, 2022
@nihal467
Copy link
Member

test approved

@gigincg
Copy link
Member

gigincg commented Jun 4, 2022

@Ashesh3 Can you make the Backend issues as well for this?

@gigincg gigincg merged commit 9f92a70 into coronasafe:develop Jun 4, 2022
@Ashesh3
Copy link
Member Author

Ashesh3 commented Jun 4, 2022

@Ashesh3 Can you make the Backend issues as well for this?

Sure, On it!

@Ashesh3
Copy link
Member Author

Ashesh3 commented Jun 8, 2022

@Ashesh3 Can you make the Backend issues as well for this?

Made it here: coronasafe/care#838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty notes and comments should not be allowed
6 participants