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

[MD] Validate length of title to be no longer than 32 characters #6452

Merged

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented Apr 15, 2024

Description

Add validation for length of title to be no longer than 32 characters in create flow. Otherwise show inline error message, and disable the buttons. This applies to both create flow and edit flow

image

image

Issues Resolved

#6433
#6432

Screenshot

iShot_2024-04-14_23.51.32.mp4

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@zhongnansu zhongnansu force-pushed the limit-title-length-in-create branch from 2868999 to 1c50e72 Compare April 15, 2024 06:58
@zhongnansu zhongnansu changed the title [MD] Validate length of title to be less or equal than 32 characters [MD] Validate length of title to be less or equal than 32 characters in create flow Apr 15, 2024
@zhongnansu zhongnansu force-pushed the limit-title-length-in-create branch 2 times, most recently from 859d2b9 to 6b051f8 Compare April 15, 2024 07:08
@zhongnansu zhongnansu changed the title [MD] Validate length of title to be less or equal than 32 characters in create flow [MD] Validate length of title to be no longer than 32 characters in create flow Apr 15, 2024
@zhongnansu zhongnansu changed the title [MD] Validate length of title to be no longer than 32 characters in create flow [MD] Validate length of title to be no longer than 32 characters Apr 15, 2024
@zhongnansu zhongnansu force-pushed the limit-title-length-in-create branch from 6b051f8 to 8f75470 Compare April 15, 2024 07:36
@zhongnansu zhongnansu marked this pull request as ready for review April 15, 2024 07:36
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.51%. Comparing base (e785fd3) to head (8f75470).
Report is 5 commits behind head on main.

❗ Current head 8f75470 differs from pull request most recent head 64ea71a. Consider uploading reports for the commit 64ea71a to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6452       +/-   ##
===========================================
+ Coverage   49.55%   62.51%   +12.95%     
===========================================
  Files        2670     2976      +306     
  Lines       54290    58663     +4373     
  Branches     8878     9553      +675     
===========================================
+ Hits        26906    36672     +9766     
+ Misses      25720    19891     -5829     
- Partials     1664     2100      +436     
Flag Coverage Δ
Windows_1 32.74% <ø> (ø)
Windows_2 55.58% <ø> (?)
Windows_3 45.10% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Outdated Show resolved Hide resolved
@zhongnansu zhongnansu force-pushed the limit-title-length-in-create branch 3 times, most recently from 650c8e3 to 043f98c Compare April 16, 2024 00:39
@@ -50,8 +50,14 @@ export const isTitleValid = (
error: '',
};
/* Title validation */
if (!title?.trim?.().length) {
if (!title.trim().length) {
Copy link
Member

Choose a reason for hiding this comment

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

Just curious whether title could be undefined. If yes, then calling trim function will throw exception. If no, then there is no concern.

Copy link
Member Author

Choose a reason for hiding this comment

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

title will not be undefined, it's initialized with empty string
that's why in the function param, we didn't declare it to be optional param

@zhongnansu zhongnansu force-pushed the limit-title-length-in-create branch from 043f98c to 64ea71a Compare April 16, 2024 05:06
@zhongnansu zhongnansu merged commit 236ec4e into opensearch-project:main Apr 16, 2024
66 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 16, 2024
…6452)

Signed-off-by: Zhongnan Su <[email protected]>
(cherry picked from commit 236ec4e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
zhongnansu pushed a commit that referenced this pull request Apr 16, 2024
…6452)

Signed-off-by: Zhongnan Su <[email protected]>
(cherry picked from commit 236ec4e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
zhongnansu pushed a commit that referenced this pull request Apr 16, 2024
…6452) (#6476)

Signed-off-by: Zhongnan Su <[email protected]>
(cherry picked from commit 236ec4e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

6 participants