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

Check team and project name validity #3034

Merged
merged 11 commits into from
Aug 13, 2018

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented Aug 10, 2018

URL of deployed dev instance (used for testing):

Steps to test:

  • got to Administration - Teams
  • try creating a new team
  • when a invalid name is entered, the request should not be send and the input field should visualize the misspelling

Issues:


  • Updated changelog
  • check team name in frontend
  • eventually change the check of project names in frontend
  • Ready for review

@MichaelBuessemeyer MichaelBuessemeyer self-assigned this Aug 10, 2018
@MichaelBuessemeyer MichaelBuessemeyer changed the title Check team name validity Check team and project name validity Aug 13, 2018
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Nice 👍 I made some small comments, though. Mostly wording related :)

@@ -99,8 +99,14 @@ class ProjectCreateView extends React.PureComponent<Props, State> {
rules: [
{
required: true,
pattern: "^[a-zA-Z0-9_-]*$",
message: "invalide project name, there are no whitespaces allowed",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "The project name must not contain whitespace." (with fullstop), since this is a user-facing message.

{
min: 3,
required: true,
message: "project name must be at least 3 characters long",
Copy link
Member

Choose a reason for hiding this comment

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

"Project name [...] ." (capitalization and full stop)

{
required: true,
pattern: "^[A-Za-z0-9\\-_\\. ß]+$",
message: "Please enter a valide team name.",
Copy link
Member

Choose a reason for hiding this comment

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

valid without e :)

const newTeam = {
name: this.state.newTeamName,
name: values["Team Name"],
Copy link
Member

Choose a reason for hiding this comment

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

This syntax (having a string instead of a variable inside the square brackets) is usually a sign that something is weird :) In this case, you should rename "Team Name" to "teamName" in your getFieldDecorator call. Then, you can access `values.teamName" here.

CHANGELOG.md Outdated
@@ -23,7 +23,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
- Added the possibility to copy volume tracings to own account
- During the import of multiple NML files, the user can select an option to automatically create a group per file so that the imported trees are organized in a hierarchy. [#2908](https://github.com/scalableminds/webknossos/pull/2908)
- Added functions to the front-end API to activate a tree and to change the color of a tree. [#2997](https://github.com/scalableminds/webknossos/pull/2997)
- When a new team is created invalid names will be directly marked in red [#2591](https://github.com/scalableminds/webknossos/issues/2591)
- When a new team or project is created invalid names will be directly marked in red. [#3034](https://github.com/scalableminds/webknossos/pull/3034)
Copy link
Member

Choose a reason for hiding this comment

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

I'd add a comma before "invalid names".

@MichaelBuessemeyer
Copy link
Contributor Author

@philippotto Please review my changes again.

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Awesome! I fine-tuned the validation message a bit. Should be good to go now 👍

@MichaelBuessemeyer MichaelBuessemeyer merged commit c98dabd into master Aug 13, 2018
@philippotto philippotto deleted the check-team-name-validity branch September 18, 2018 13:19
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.

Check Team and Project Name Validity in Frontend
2 participants