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

Add validation for new shared folder #1390

Merged
merged 18 commits into from
Aug 4, 2021

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Aug 2, 2021

closes #1293

  • refactor to use only 1 name validator (it was redundant with only small error changes)
  • remove the specific whitespace error while trim handles this nicely

@Tbaut Tbaut added the Status: Review Needed 👀 Added to PRs when they need more review label Aug 2, 2021
@render
Copy link

render bot commented Aug 2, 2021

@render
Copy link

render bot commented Aug 2, 2021

@render
Copy link

render bot commented Aug 2, 2021

@github-actions github-actions bot added the Type: Bug Fix Added to PRs if they are addressing a bug label Aug 2, 2021
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Works great!
We just need to add the validator to create shared folder during share file!

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 3, 2021

Ah thanks I thought I had that in. I'll check it out.

@Tbaut Tbaut requested a review from tanmoyAtb August 3, 2021 17:34
@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 3, 2021

Should be good now @tanmoyAtb

@Tbaut Tbaut enabled auto-merge (squash) August 3, 2021 17:35
@asnaith
Copy link
Member

asnaith commented Aug 3, 2021

Looking good but I noticed these changes also stop the user from having a legitimate space in the name, for example "My Share" is no longer possible.

🟩 User cannot create a folder with name being white space only
🟩 User cannot create a folder without entering any characters for the name
🟩 User cannot create a folder with name exceeding 65 characters
🟩 User cannot create a folder with name containing "/"
🟥 User should be able to create folder with space in the name eg "Best Share Folder"

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 3, 2021

Nice catch! will fix that.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 4, 2021

ok should be good to go now

@FSM1
Copy link
Contributor

FSM1 commented Aug 4, 2021

This PR seems to be breaking the Rename a file test

1) File management
 |        desktop
 |          can rename a file with error handling:
 |      AssertionError: Timed out retrying after 4000ms: Expected to find element: `[data-cy=rename-form] span.minimal.error`, but never found it.
 |       at Context.eval (http://localhost:3000/__cypress/tests?p=cypress/tests/file-management-spec.ts:495:56)

image

Copy link
Contributor

@FSM1 FSM1 left a comment

Choose a reason for hiding this comment

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

great stuff :)

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 4, 2021

I'll stop there, thanks a lot for the thought review, this was really needed, and I'm super glad the tests caught a bug, nice catch @FSM1 !

I think this will conflict with Tanmoy's PR, I'll merge his before

@Tbaut Tbaut disabled auto-merge August 4, 2021 11:56
…ub.com:ChainSafe/files-ui into fix/tbaut-validate-shared-folder-creation-1293
@Tbaut Tbaut merged commit 2db0874 into dev Aug 4, 2021
@Tbaut Tbaut deleted the fix/tbaut-validate-shared-folder-creation-1293 branch August 4, 2021 12:10
@FSM1 FSM1 mentioned this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed 👀 Added to PRs when they need more review Type: Bug Fix Added to PRs if they are addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use some validation for Shared folder creation
6 participants