From 463d17ace1ea476a53d6f4a5c3022feaa5057cc0 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Fri, 20 Oct 2017 15:19:21 +0200 Subject: [PATCH 1/8] Add first version of reusable validation form elements Implemented on new team page. --- .../components/ValidatedErrorHelpBlock.jsx | 60 +++++++ .../validation/components/ValidatedForm.jsx | 113 ++++++++++++ .../components/ValidatedFormControl.jsx | 70 ++++++++ .../components/ValidatedFormGroup.jsx | 41 +++++ .../components/ValidatedSubmitButton.jsx | 23 +++ .../src/components/validation/index.js | 5 + .../validation/validators/text_validators.js | 60 +++++++ app/javascript/src/config/locales/messages.js | 6 + .../teams/new/components/NameFormControl.jsx | 4 +- .../SettingsPage/scenes/teams/new/index.jsx | 167 +++++------------- 10 files changed, 422 insertions(+), 127 deletions(-) create mode 100644 app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx create mode 100644 app/javascript/src/components/validation/components/ValidatedForm.jsx create mode 100644 app/javascript/src/components/validation/components/ValidatedFormControl.jsx create mode 100644 app/javascript/src/components/validation/components/ValidatedFormGroup.jsx create mode 100644 app/javascript/src/components/validation/components/ValidatedSubmitButton.jsx create mode 100644 app/javascript/src/components/validation/index.js create mode 100644 app/javascript/src/components/validation/validators/text_validators.js diff --git a/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx new file mode 100644 index 000000000..1dc37a432 --- /dev/null +++ b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx @@ -0,0 +1,60 @@ +import React, { Component } from "react"; +import { HelpBlock } from "react-bootstrap"; +import { FormattedMessage } from "react-intl"; +import PropTypes from "prop-types"; +import styled from "styled-components"; +import shortid from "shortid"; + +const MyHelpBlock = styled(HelpBlock)` + & > span { + margin-right: 5px; + } +`; + +class ValidatedErrorHelpBlock extends Component { + static renderErrorMessage(error) { + const key = shortid.generate(); + if (error.intl) { + return ( + + ); + } + return {error.message}; + } + + constructor(props) { + super(props); + + this.cleanProps = this.cleanProps.bind(this); + this.cleanProps = this.cleanProps.bind(this); + } + + cleanProps() { + // Remove additional props from the props + const { tag, ...cleanProps } = this.props; + return cleanProps; + } + + render() { + const errors = this.context.errors(this.props.tag) || []; + return ( + + {errors.map((error) => ValidatedErrorHelpBlock.renderErrorMessage(error))} + + ); + } +} + +ValidatedErrorHelpBlock.propTypes = { + tag: PropTypes.string.isRequired +}; + +ValidatedErrorHelpBlock.contextTypes = { + errors: PropTypes.func +} + +export default ValidatedErrorHelpBlock; \ No newline at end of file diff --git a/app/javascript/src/components/validation/components/ValidatedForm.jsx b/app/javascript/src/components/validation/components/ValidatedForm.jsx new file mode 100644 index 000000000..7005decdc --- /dev/null +++ b/app/javascript/src/components/validation/components/ValidatedForm.jsx @@ -0,0 +1,113 @@ +import React, { Component } from "react"; +import update from "immutability-helper"; +import PropTypes from "prop-types"; +import _ from "lodash"; + +class ValidatedForm extends Component { + constructor(props) { + super(props); + + this.state = {} + + this.setErrors = this.setErrors.bind(this); + this.setErrorsForTag = this.setErrorsForTag.bind(this); + this.errors = this.errors.bind(this); + this.hasAnyError = this.hasAnyError.bind(this); + this.hasErrorForTag = this.hasErrorForTag.bind(this); + this.addErrorsForTag = this.addErrorsForTag.bind(this); + this.clearErrorsForTag = this.clearErrorsForTag.bind(this); + this.clearErrors = this.clearErrors.bind(this); + } + + getChildContext() { + // Pass functions downstream via context + return { + setErrors: this.setErrors, + setErrorsForTag: this.setErrorsForTag, + errors: this.errors, + hasAnyError: this.hasAnyError, + hasErrorForTag: this.hasErrorForTag, + addErrorsForTag: this.addErrorsForTag, + clearErrorsForTag: this.clearErrorsForTag, + clearErrors: this.clearErrors + }; + } + + setErrors(errors) { + // This method is quite smart, in the sense that accepts either + // errors in 3 shapes: localized error messages ({}), + // unlocalized error messages ({}), or mere strings (unlocalized) + const newState = {}; + _.entries(errors).forEach(([key, value]) => { + const arr = _.isString(value) ? [value] : value; + newState[key] = arr.map((el) => _.isString(el) ? { message: el } : el); + }); + this.setState(newState); + } + + setErrorsForTag(tag, errors) { + const newState = update(this.state, { [tag]: { $set: errors } }); + this.setState(newState); + } + + errors(tag) { + return this.state[tag]; + } + + hasAnyError() { + return _.values(this.state) && + _.flatten(_.values(this.state)).length > 0; + } + + hasErrorForTag(tag) { + return _.has(this.state, tag) && this.state[tag].length > 0; + } + + addErrorsForTag(tag, errors) { + let newState; + if (_.has(this.state, tag)) { + newState = update(this.state, { [tag]: { $push: errors } }); + } else { + newState = update(this.state, { [tag]: { $set: errors } }); + } + this.setState(newState); + } + + clearErrorsForTag(tag) { + const newState = update(this.state, { [tag]: { $set: [] } }); + this.setState(newState); + } + + clearErrors() { + this.setState({}); + } + + render() { + return ( +
+ {this.props.children} +
+ ); + } +} + +ValidatedForm.propTypes = { + children: PropTypes.node +} + +ValidatedForm.defaultProps = { + children: undefined +} + +ValidatedForm.childContextTypes = { + setErrors: PropTypes.func, + setErrorsForTag: PropTypes.func, + errors: PropTypes.func, + hasAnyError: PropTypes.func, + hasErrorForTag: PropTypes.func, + addErrorsForTag: PropTypes.func, + clearErrorsForTag: PropTypes.func, + clearErrors: PropTypes.func +} + +export default ValidatedForm; \ No newline at end of file diff --git a/app/javascript/src/components/validation/components/ValidatedFormControl.jsx b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx new file mode 100644 index 000000000..c6c54dc72 --- /dev/null +++ b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx @@ -0,0 +1,70 @@ +import React, { Component } from "react"; +import { FormControl } from "react-bootstrap"; +import PropTypes from "prop-types"; + +class ValidatedFormControl extends Component { + constructor(props) { + super(props); + + this.handleChange = this.handleChange.bind(this); + this.cleanProps = this.cleanProps.bind(this); + } + + handleChange(e) { + const tag = this.props.tag; + const messageIds = this.props.messageIds; + const value = e.target.value; + + // Pass-through "original" onChange + if (_.has(this.props, "onChange")) { + this.props.onChange(e); + } + + // Validate the field + let errors = []; + this.props.validatorsOnChange.forEach((validator) => { + errors = errors.concat(validator(value, messageIds)); + }); + this.context.setErrorsForTag(tag, errors); + } + + cleanProps() { + // Remove additional props from the props + const { + tag, + messageIds, + validatorsOnChange, + onChange, + ...cleanProps + } = this.props; + return cleanProps; + } + + render() { + return ( + + ); + } +} + +ValidatedFormControl.propTypes = { + tag: PropTypes.string.isRequired, + messageIds: PropTypes.objectOf(PropTypes.string), + validatorsOnChange: PropTypes.arrayOf(PropTypes.func), + onChange: PropTypes.func +} + +ValidatedFormControl.defaultProps = { + messageIds: {}, + validatorsOnChange: [], + onChange: undefined +} + +ValidatedFormControl.contextTypes = { + setErrorsForTag: PropTypes.func +} + +export default ValidatedFormControl; \ No newline at end of file diff --git a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx new file mode 100644 index 000000000..e8a5e11d8 --- /dev/null +++ b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx @@ -0,0 +1,41 @@ +import React, { Component } from "react"; +import { FormGroup } from "react-bootstrap"; +import PropTypes from "prop-types"; + +class ValidatedFormGroup extends Component { + constructor(props) { + super(props); + + this.cleanProps = this.cleanProps.bind(this); + } + + cleanProps() { + // Remove additional props from the props + const { tag, ...cleanProps } = this.props; + return cleanProps; + } + + render() { + const hasError = this.context.hasErrorForTag(this.props.tag); + const formGroupClass = `form-group ${hasError ? " has-error" : ""}`; + const validationState = hasError ? "error" : null; + + return ( + + ); + } +} + +ValidatedFormGroup.propTypes = { + tag: PropTypes.string.isRequired +} + +ValidatedFormGroup.contextTypes = { + hasErrorForTag: PropTypes.func +} + +export default ValidatedFormGroup; \ No newline at end of file diff --git a/app/javascript/src/components/validation/components/ValidatedSubmitButton.jsx b/app/javascript/src/components/validation/components/ValidatedSubmitButton.jsx new file mode 100644 index 000000000..d2afa6659 --- /dev/null +++ b/app/javascript/src/components/validation/components/ValidatedSubmitButton.jsx @@ -0,0 +1,23 @@ +import React from "react"; +import { Button } from "react-bootstrap"; +import PropTypes from "prop-types"; + +const ValidatedSubmitButton = (props, context) => + +; + +ValidatedSubmitButton.propTypes = { + children: PropTypes.node +} + +ValidatedSubmitButton.defaultProps = { + children: undefined +} + +ValidatedSubmitButton.contextTypes = { + hasAnyError: PropTypes.func +} + +export default ValidatedSubmitButton; \ No newline at end of file diff --git a/app/javascript/src/components/validation/index.js b/app/javascript/src/components/validation/index.js new file mode 100644 index 000000000..991f010a4 --- /dev/null +++ b/app/javascript/src/components/validation/index.js @@ -0,0 +1,5 @@ +export {default as ValidatedErrorHelpBlock} from "./components/ValidatedErrorHelpBlock"; +export {default as ValidatedForm} from "./components/ValidatedForm"; +export {default as ValidatedFormControl} from "./components/ValidatedFormControl"; +export {default as ValidatedFormGroup} from "./components/ValidatedFormGroup"; +export {default as ValidatedSubmitButton} from "./components/ValidatedSubmitButton"; \ No newline at end of file diff --git a/app/javascript/src/components/validation/validators/text_validators.js b/app/javascript/src/components/validation/validators/text_validators.js new file mode 100644 index 000000000..2a6c61aa1 --- /dev/null +++ b/app/javascript/src/components/validation/validators/text_validators.js @@ -0,0 +1,60 @@ +import _ from "lodash"; +import { + NAME_MIN_LENGTH, + NAME_MAX_LENGTH, + TEXT_MAX_LENGTH +} from "../../../config/constants/numeric"; + +export const nameMinLengthValidator = (value, messageIds = {}) => { + const messageId = _.has(messageIds, "text_too_short") ? + messageIds.text_too_short : + "validators.text_validators.text_too_short"; + + + if (value.length < NAME_MIN_LENGTH) { + return [{ + intl: true, + messageId, + values: { min_length: NAME_MIN_LENGTH } + }]; + } + return []; +}; + +export const nameMaxLengthValidator = (value, messageIds = {}) => { + const messageId = _.has(messageIds, "text_too_long") ? + messageIds.text_too_long : + "validators.text_validators.text_too_long"; + + if (value.length > NAME_MAX_LENGTH) { + return [{ + intl: true, + messageId, + values:{ max_length: NAME_MAX_LENGTH } + }]; + } + return []; +}; + +export const nameLengthValidator = (value, messageIds = {}) => { + const res = nameMinLengthValidator(value, messageIds); + if (res.length > 0) { + return res; + } + return nameMaxLengthValidator(value, messageIds); +}; + +export const textMaxLengthValidator = (value, messageIds = {}) => { + const messageId = _.has(messageIds, "text_too_long") ? + messageIds.text_too_long : + "validators.text_validators.text_too_long"; + + if (value.length > TEXT_MAX_LENGTH) { + return [{ + intl: true, + messageId, + values: { max_length: TEXT_MAX_LENGTH } + }]; + } + return []; +}; \ No newline at end of file diff --git a/app/javascript/src/config/locales/messages.js b/app/javascript/src/config/locales/messages.js index 8be4af078..fa87b5bf9 100644 --- a/app/javascript/src/config/locales/messages.js +++ b/app/javascript/src/config/locales/messages.js @@ -16,6 +16,12 @@ export default { all_teams_page: "SciNote | Settings | Teams", new_team_page: "SciNote | Settings | Teams | New" }, + validators: { + text_validators: { + text_too_short: "is too short (minimum is {min_length} characters)", + text_too_long: "is too long (maximum is {max_length} characters)" + } + }, error_messages: { text_too_short: "is too short (minimum is {min_length} characters)", text_too_long: "is too long (maximum is {max_length} characters)", diff --git a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/components/NameFormControl.jsx b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/components/NameFormControl.jsx index cb82deab0..6b796b169 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/components/NameFormControl.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/components/NameFormControl.jsx @@ -1,13 +1,13 @@ import React from "react"; import {defineMessages, injectIntl, intlShape} from 'react-intl'; -import { FormControl } from "react-bootstrap"; +import { ValidatedFormControl } from "../../../../../../components/validation"; const messages = defineMessages({ placeholder: { id: "settings_page.new_team.name_placeholder" } }); const NameFormControl = ({ intl, ...props }) => - { name: "", description: "" }, - formErrors: { - name: "", - description: "" - }, redirectTo: "" }; (this: any).onSubmit = this.onSubmit.bind(this); - (this: any).validateField = this.validateField.bind(this); (this: any).handleChange = this.handleChange.bind(this); (this: any).renderTeamNameFormGroup = this.renderTeamNameFormGroup.bind( this @@ -100,138 +94,62 @@ class SettingsNewTeam extends Component { .then(response => { // Redirect to the new team page this.props.getTeamsList(); - (this: any).newState = { ...this.state }; - (this: any).newState = update((this: any).newState, { + + const newState = update((this: any).state, { redirectTo: { $set: SETTINGS_TEAM_ROUTE.replace(":id", response.details.id) } }); - (this: any).setState((this: any).newState); + (this: any).setState(newState); }) .catch(er => { // Display errors - (this: any).newState = { ...this.state }; - ["name", "description"].forEach(el => { - if (er.response.data.details[el]) { - (this: any).newState = update((this: any).newState, { - formErrors: { - name: { $set: {er.response.data.details[el]} } - } - }); - } - }); - (this: any).setState((this: any).newState); + (this: any).newTeamForm.setErrors(er.response.data.details); }); } - validateField(key: string, value: string) { - let errorMessage; - if (key === "name") { - errorMessage = ""; - - if (value.length < NAME_MIN_LENGTH) { - errorMessage = ( - - ); - } else if (value.length > NAME_MAX_LENGTH) { - errorMessage = ( - - ); - } - - (this: any).newState = update((this: any).newState, { - formErrors: { name: { $set: errorMessage } } - }); - } else if (key === "description") { - errorMessage = ""; - - if (value.length > TEXT_MAX_LENGTH) { - errorMessage = ( - - ); - } - - (this: any).newState = update((this: any).newState, { - formErrors: { description: { $set: errorMessage } } - }); - } - } - - handleChange(e: SyntheticInputEvent): void { - const key = e.target.name; + handleChange(e: SyntheticInputEvent, tag: string): void { const value = e.target.value; - (this: any).newState = { ...this.state }; - - // Update value in the state - (this: any).newState = update((this: any).newState, { - team: { [key]: { $set: value } } + const newState = update((this: any).state, { + team: { [tag]: { $set: value } } }); - - // Validate the input - (this: any).validateField(key, value); - - // Refresh state - (this: any).setState((this: any).newState); + (this: any).setState(newState); } renderTeamNameFormGroup() { - const formGroupClass = this.state.formErrors.name - ? "form-group has-error" - : "form-group"; - const validationState = this.state.formErrors.name ? "error" : null; return ( - + this.handleChange(e, "name")} /> - {this.state.formErrors.name} - + + ); } renderTeamDescriptionFormGroup() { - const formGroupClass = this.state.formErrors.description - ? "form-group has-error" - : "form-group"; - const validationState = this.state.formErrors.description ? "error" : null; return ( - + - this.handleChange(e, "description")} /> - - {this.state.formErrors.description} - + + ); } @@ -241,10 +159,6 @@ class SettingsNewTeam extends Component { return ; } - const btnDisabled = - !_.isEmpty(this.state.formErrors.name) || - !_.isEmpty(this.state.formErrors.description); - return ( @@ -259,7 +173,11 @@ class SettingsNewTeam extends Component { -
+ { (this: any).newTeamForm = f; }} + style={{ maxWidth: "500px" }} + > {this.renderTeamNameFormGroup()} @@ -274,20 +192,19 @@ class SettingsNewTeam extends Component { - + - +
); From ceb11c143fc2e0691a557f9a2d3415fc8a4cb4b5 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Mon, 23 Oct 2017 12:55:32 +0200 Subject: [PATCH 2/8] Change edit team name & description modals to use new validation --- .../components/UpdateTeamDescriptionModal.jsx | 109 +++++++---------- .../team/components/UpdateTeamNameModal.jsx | 115 ++++++++---------- 2 files changed, 98 insertions(+), 126 deletions(-) diff --git a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx index ab0e08d54..fc54f563b 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx @@ -3,55 +3,40 @@ import PropTypes, { bool, number, string, func } from "prop-types"; import { Modal, Button, - FormGroup, ControlLabel, FormControl, - HelpBlock } from "react-bootstrap"; import { FormattedMessage } from "react-intl"; -import _ from "lodash"; -import styled from "styled-components"; import axios from "../../../../../config/axios"; +import { + ValidatedForm, + ValidatedFormGroup, + ValidatedFormControl, + ValidatedErrorHelpBlock, + ValidatedSubmitButton +} from "../../../../../components/validation"; +import { + textMaxLengthValidator +} from "../../../../../components/validation/validators/text_validators"; -import { TEXT_MAX_LENGTH } from "../../../../../config/constants/numeric"; import { TEAM_UPDATE_PATH } from "../../../../../config/api_endpoints"; -import { COLOR_APPLE_BLOSSOM } from "../../../../../config/constants/colors"; - -const StyledHelpBlock = styled(HelpBlock)`color: ${COLOR_APPLE_BLOSSOM};`; class UpdateTeamDescriptionModal extends Component { constructor(props) { super(props); - this.state = { errorMessage: "", description: "" }; + this.state = { description: "" }; this.onCloseModal = this.onCloseModal.bind(this); this.updateDescription = this.updateDescription.bind(this); this.handleDescription = this.handleDescription.bind(this); - this.getValidationState = this.getValidationState.bind(this); } onCloseModal() { - this.setState({ errorMessage: "", description: "" }); + this.setState({ description: "" }); this.props.hideModal(); } - getValidationState() { - return this.state.errorMessage.length > 0 ? "error" : null; - } - handleDescription(el) { - const { value } = el.target; - if (value.length > TEXT_MAX_LENGTH) { - this.setState({ - errorMessage: ( - - ) - }); - } else { - this.setState({ errorMessage: "", description: value }); - } + this.setState({ description: el.target.value }); } updateDescription() { @@ -74,40 +59,40 @@ class UpdateTeamDescriptionModal extends Component { render() { return ( - - - - - - - - - - - - - {this.state.errorMessage} - - - - - - + { this.form = f; }}> + + + + + + + + + + + + + + + + + + + + + + ); } diff --git a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx index eff100622..2bed07904 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx @@ -3,55 +3,40 @@ import PropTypes, { bool, number, string, func } from "prop-types"; import { Modal, Button, - FormGroup, ControlLabel, FormControl, - HelpBlock } from "react-bootstrap"; import { FormattedMessage } from "react-intl"; -import _ from "lodash"; -import styled from "styled-components"; import axios from "../../../../../config/axios"; +import { + ValidatedForm, + ValidatedFormGroup, + ValidatedFormControl, + ValidatedErrorHelpBlock, + ValidatedSubmitButton +} from "../../../../../components/validation"; +import { + nameLengthValidator +} from "../../../../../components/validation/validators/text_validators"; -import { NAME_MAX_LENGTH } from "../../../../../config/constants/numeric"; import { TEAM_UPDATE_PATH } from "../../../../../config/api_endpoints"; -import { COLOR_APPLE_BLOSSOM } from "../../../../../config/constants/colors"; - -const StyledHelpBlock = styled(HelpBlock)`color: ${COLOR_APPLE_BLOSSOM};`; class UpdateTeamNameModal extends Component { constructor(props) { super(props); - this.state = { errorMessage: "", name: props.team.name }; + this.state = { name: props.team.name }; this.onCloseModal = this.onCloseModal.bind(this); this.updateName = this.updateName.bind(this); this.handleName = this.handleName.bind(this); - this.getValidationState = this.getValidationState.bind(this); } onCloseModal() { - this.setState({ errorMessage: "", name: "" }); + this.setState({ name: "" }); this.props.hideModal(); } - getValidationState() { - return this.state.errorMessage.length > 0 ? "error" : null; - } - - handleName(el) { - const { value } = el.target; - if (value.length > NAME_MAX_LENGTH) { - this.setState({ - errorMessage: ( - - ) - }); - } else { - this.setState({ errorMessage: "", name: value }); - } + handleName(e) { + this.setState({ name: e.target.value }); } updateName() { @@ -68,46 +53,48 @@ class UpdateTeamNameModal extends Component { this.props.updateTeamCallback(response.data.team); this.onCloseModal(); }) - .catch(error => this.setState({ errorMessage: error.message })); + .catch(error => { + this.form.setErrorsForTag("name", [error.message]); + }); } render() { return ( - - - - - - - - - - - - - {this.state.errorMessage} - - - - - - + { this.form = f; }}> + + + + + + + + + + + + + + + + + + + + + + ); } From c026b60790b1a0c2161b79e559628c443278430d Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Mon, 23 Oct 2017 17:45:57 +0200 Subject: [PATCH 3/8] Update Profile page to use validation --- .../client_api/users/users_controller.rb | 39 +- .../components/ValidatedErrorHelpBlock.jsx | 1 - .../validation/components/ValidatedForm.jsx | 18 +- .../components/ValidatedFormControl.jsx | 2 +- .../validation/validators/text_validators.js | 76 +++- app/javascript/src/config/locales/messages.js | 4 +- .../profile/components/InputEnabled.jsx | 430 +++++++----------- app/services/client_api/user_service.rb | 28 -- .../client_api/users/update_service.rb | 53 +++ config/locales/en.yml | 6 +- features/settings_page/profile.feature | 2 +- spec/services/client_api/user_service_spec.rb | 80 ---- .../client_api/users/update_service_spec.rb | 79 ++++ 13 files changed, 414 insertions(+), 404 deletions(-) delete mode 100644 app/services/client_api/user_service.rb create mode 100644 app/services/client_api/users/update_service.rb delete mode 100644 spec/services/client_api/user_service_spec.rb create mode 100644 spec/services/client_api/users/update_service_spec.rb diff --git a/app/controllers/client_api/users/users_controller.rb b/app/controllers/client_api/users/users_controller.rb index 98c663c40..89d148d76 100644 --- a/app/controllers/client_api/users/users_controller.rb +++ b/app/controllers/client_api/users/users_controller.rb @@ -57,19 +57,21 @@ module ClientApi end def update - user_service = ClientApi::UserService.new( + service = ClientApi::Users::UpdateService.new( current_user: current_user, params: user_params ) - if user_service.update_user! + result = service.execute + + if result[:status] == :success bypass_sign_in(current_user) success_response else - unsuccess_response(current_user.errors.full_messages, - :unprocessable_entity) + error_response( + message: result[:message], + details: service.user.errors + ) end - rescue CustomUserError => error - unsuccess_response(error.to_s) end private @@ -84,22 +86,35 @@ module ClientApi :system_message_email_notification) end - def success_response(template = nil, locals = nil) + def success_response(args = {}) + template = args.fetch(:template) { nil } + locals = args.fetch(:locals) { {} } + details = args.fetch(:details) { {} } + respond_to do |format| format.json do - if template && locals - render template: template, status: :ok, locals: locals + if template + render template: template, + status: :ok, + locals: locals else - render json: {}, status: :ok + render json: { details: details }, status: :ok end end end end - def unsuccess_response(message, status = :unprocessable_entity) + def error_response(args = {}) + message = args.fetch(:message) { t('client_api.generic_error_message') } + details = args.fetch(:details) { {} } + status = args.fetch(:status) { :unprocessable_entity } + respond_to do |format| format.json do - render json: { message: message }, + render json: { + message: message, + details: details + }, status: status end end diff --git a/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx index 1dc37a432..2e9b1afa8 100644 --- a/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx +++ b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx @@ -30,7 +30,6 @@ class ValidatedErrorHelpBlock extends Component { super(props); this.cleanProps = this.cleanProps.bind(this); - this.cleanProps = this.cleanProps.bind(this); } cleanProps() { diff --git a/app/javascript/src/components/validation/components/ValidatedForm.jsx b/app/javascript/src/components/validation/components/ValidatedForm.jsx index 7005decdc..5b15a251a 100644 --- a/app/javascript/src/components/validation/components/ValidatedForm.jsx +++ b/app/javascript/src/components/validation/components/ValidatedForm.jsx @@ -4,6 +4,14 @@ import PropTypes from "prop-types"; import _ from "lodash"; class ValidatedForm extends Component { + static parseErrors(errors) { + // This method is quite smart, in the sense that accepts either + // errors in 3 shapes: localized error messages ({}), + // unlocalized error messages ({}), or mere strings (unlocalized) + const arr = _.isString(errors) ? [errors] : errors; + return arr.map((el) => _.isString(el) ? { message: el } : el); + } + constructor(props) { super(props); @@ -34,19 +42,17 @@ class ValidatedForm extends Component { } setErrors(errors) { - // This method is quite smart, in the sense that accepts either - // errors in 3 shapes: localized error messages ({}), - // unlocalized error messages ({}), or mere strings (unlocalized) const newState = {}; _.entries(errors).forEach(([key, value]) => { - const arr = _.isString(value) ? [value] : value; - newState[key] = arr.map((el) => _.isString(el) ? { message: el } : el); + newState[key] = ValidatedForm.parseErrors(value); }); this.setState(newState); } setErrorsForTag(tag, errors) { - const newState = update(this.state, { [tag]: { $set: errors } }); + const newState = update(this.state, { + [tag]: { $set: ValidatedForm.parseErrors(errors) } + }); this.setState(newState); } diff --git a/app/javascript/src/components/validation/components/ValidatedFormControl.jsx b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx index c6c54dc72..6297b4ce7 100644 --- a/app/javascript/src/components/validation/components/ValidatedFormControl.jsx +++ b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx @@ -16,7 +16,7 @@ class ValidatedFormControl extends Component { const value = e.target.value; // Pass-through "original" onChange - if (_.has(this.props, "onChange")) { + if (_.has(this.props, "onChange") && this.props.onChange !== undefined) { this.props.onChange(e); } diff --git a/app/javascript/src/components/validation/validators/text_validators.js b/app/javascript/src/components/validation/validators/text_validators.js index 2a6c61aa1..d2ad5233f 100644 --- a/app/javascript/src/components/validation/validators/text_validators.js +++ b/app/javascript/src/components/validation/validators/text_validators.js @@ -2,15 +2,18 @@ import _ from "lodash"; import { NAME_MIN_LENGTH, NAME_MAX_LENGTH, - TEXT_MAX_LENGTH + TEXT_MAX_LENGTH, + PASSWORD_MIN_LENGTH, + PASSWORD_MAX_LENGTH, + USER_INITIALS_MAX_LENGTH } from "../../../config/constants/numeric"; +import { EMAIL_REGEX } from "../../../config/constants/strings"; export const nameMinLengthValidator = (value, messageIds = {}) => { const messageId = _.has(messageIds, "text_too_short") ? messageIds.text_too_short : "validators.text_validators.text_too_short"; - if (value.length < NAME_MIN_LENGTH) { return [{ intl: true, @@ -44,6 +47,20 @@ export const nameLengthValidator = (value, messageIds = {}) => { return nameMaxLengthValidator(value, messageIds); }; +export const textBlankValidator = (value, messageIds = {}) => { + const messageId = _.has(messageIds, "text_blank") ? + messageIds.text_blank : + "validators.text_validators.text_blank"; + + if (value.length === 0) { + return [{ + intl: true, + messageId + }]; + } + return []; +} + export const textMaxLengthValidator = (value, messageIds = {}) => { const messageId = _.has(messageIds, "text_too_long") ? messageIds.text_too_long : @@ -57,4 +74,59 @@ export const textMaxLengthValidator = (value, messageIds = {}) => { }]; } return []; +}; + +export const passwordLengthValidator = (value, messageIds = {}) => { + const messageIdTooShort = _.has(messageIds, "text_too_short") ? + messageIds.text_too_short : + "validators.text_validators.text_too_short"; + const messageIdTooLong = _.has(messageIds, "text_too_long") ? + messageIds.text_too_long : + "validators.text_validators.text_too_long"; + + if (value.length < PASSWORD_MIN_LENGTH) { + return [{ + intl: true, + messageId: messageIdTooShort, + values:{ min_length: PASSWORD_MIN_LENGTH } + }]; + } else if (value.length > PASSWORD_MAX_LENGTH) { + return [{ + intl: true, + messageId: messageIdTooLong, + values:{ max_length: PASSWORD_MAX_LENGTH } + }]; + } + return []; +}; + +export const userInitialsMaxLengthValidator = (value, messageIds = {}) => { + const messageId = _.has(messageIds, "text_too_long") ? + messageIds.text_too_long : + "validators.text_validators.text_too_long"; + + if (value.length > USER_INITIALS_MAX_LENGTH) { + return [{ + intl: true, + messageId, + values: { max_length: USER_INITIALS_MAX_LENGTH } + }]; + } + return []; +}; + +export const emailValidator = (value, messageIds = {}) => { + const res = textBlankValidator(value, messageIds); + if (res.length > 0) { + return res; + } + + const messageId = _.has(messageIds, "invalid_email") ? + messageIds.invalid_email : + "validators.text_validators.invalid_email"; + + if (!EMAIL_REGEX.test(value)) { + return [{ intl: true, messageId }]; + } + return []; }; \ No newline at end of file diff --git a/app/javascript/src/config/locales/messages.js b/app/javascript/src/config/locales/messages.js index fa87b5bf9..3cbbdeb25 100644 --- a/app/javascript/src/config/locales/messages.js +++ b/app/javascript/src/config/locales/messages.js @@ -19,7 +19,9 @@ export default { validators: { text_validators: { text_too_short: "is too short (minimum is {min_length} characters)", - text_too_long: "is too long (maximum is {max_length} characters)" + text_too_long: "is too long (maximum is {max_length} characters)", + text_blank: "can't be blank", + invalid_email: "invalid email" } }, error_messages: { diff --git a/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx b/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx index 04f087031..7efacc543 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx @@ -4,29 +4,35 @@ import { string, func } from "prop-types"; import styled from "styled-components"; import { FormattedMessage, FormattedHTMLMessage } from "react-intl"; import { - FormGroup, - FormControl, ControlLabel, Button, ButtonToolbar, - HelpBlock } from "react-bootstrap"; +import update from "immutability-helper"; import { updateUser } from "../../../../../services/api/users_api"; import { transformName } from "../../../../../services/helpers/string_helper"; import { addAlert } from "../../../../../components/actions/AlertsActions"; import { BORDER_LIGHT_COLOR, - COLOR_APPLE_BLOSSOM } from "../../../../../config/constants/colors"; import { ENTER_KEY_CODE, - USER_INITIALS_MAX_LENGTH, - NAME_MAX_LENGTH, - PASSWORD_MAX_LENGTH, - PASSWORD_MIN_LENGTH } from "../../../../../config/constants/numeric"; -import { EMAIL_REGEX } from "../../../../../config/constants/strings"; +import { + ValidatedForm, + ValidatedFormGroup, + ValidatedFormControl, + ValidatedErrorHelpBlock, + ValidatedSubmitButton +} from "../../../../../components/validation"; +import { + textBlankValidator, + nameMaxLengthValidator, + passwordLengthValidator, + userInitialsMaxLengthValidator, + emailValidator +} from "../../../../../components/validation/validators/text_validators"; const StyledInputEnabled = styled.div` border: 1px solid ${BORDER_LIGHT_COLOR}; @@ -38,10 +44,6 @@ const StyledInputEnabled = styled.div` } `; -const StyledHelpBlock = styled(HelpBlock)` - color: ${COLOR_APPLE_BLOSSOM}; -`; - class InputEnabled extends Component { constructor(props) { super(props); @@ -49,30 +51,16 @@ class InputEnabled extends Component { this.state = { value: this.props.inputValue === "********" ? "" : this.props.inputValue, current_password: "", - password_confirmation: "", - errorMessage: "" + password_confirmation: "" }; - this.handleChange = this.handleChange.bind(this); - this.handlePasswordConfirmation = this.handlePasswordConfirmation.bind( - this - ); this.handleKeyPress = this.handleKeyPress.bind(this); - this.confirmationField = this.confirmationField.bind(this); - this.handleSubmit = this.handleSubmit.bind(this); - this.getValidationState = this.getValidationState.bind(this); - this.handleFullNameValidation = this.handleFullNameValidation.bind(this); - this.handleEmailValidation = this.handleEmailValidation.bind(this); - this.handleInitialsValidation = this.handleInitialsValidation.bind(this); - this.handlePasswordConfirmationValidation = this.handlePasswordConfirmationValidation.bind( - this - ); + this.handleChange = this.handleChange.bind(this); this.handleCurrentPassword = this.handleCurrentPassword.bind(this); - this.handleFileChange = this.handleFileChange.bind(this); - } - - getValidationState() { - return this.state.errorMessage.length > 0 ? "error" : null; + this.handlePasswordConfirmation = this.handlePasswordConfirmation.bind(this); + this.handleSubmit = this.handleSubmit.bind(this); + this.inputField = this.inputField.bind(this); + this.confirmationField = this.confirmationField.bind(this); } handleKeyPress(event) { @@ -83,170 +71,30 @@ class InputEnabled extends Component { } handleChange(event) { - event.preventDefault(); - switch (this.props.dataField) { - case "full_name": - this.handleFullNameValidation(event); - break; - case "email": - this.handleEmailValidation(event); - break; - case "initials": - this.handleInitialsValidation(event); - break; - case "password": - this.handlePasswordValidation(event); - break; - case "avatar": - this.handleFileChange(event); - break; - default: - this.setState({ value: event.target.value, errorMessage: "" }); - } - } - - handleFileChange(event) { - this.setState({ value: event.currentTarget.files[0], errorMessage: "" }); - } - - handlePasswordConfirmation(event) { - const { value } = event.target; - if (value.length === 0) { - this.setState({ - password_confirmation: value, - errorMessage: - }); - } - this.setState({ password_confirmation: value }); - } - - handleFullNameValidation(event) { - const { value } = event.target; - if (value.length > NAME_MAX_LENGTH) { - this.setState({ - value, - errorMessage: ( - - ) - }); - } else if (value.length === 0) { - this.setState({ - value, - errorMessage: - }); + let newVal; + if (this.props.dataField === "avatar") { + newVal = event.currentTarget.files[0]; } else { - this.setState({ value, errorMessage: "" }); - } - } - - handleEmailValidation(event) { - const { value } = event.target; - if (!EMAIL_REGEX.test(value)) { - this.setState({ - value, - errorMessage: - }); - } else if (value.length === 0) { - this.setState({ - value, - errorMessage: - }); - } else { - this.setState({ value, errorMessage: "" }); - } - } - - handleInitialsValidation(event) { - const { value } = event.target; - if (value.length > USER_INITIALS_MAX_LENGTH) { - this.setState({ - value, - errorMessage: ( - - ) - }); - } else if (value.length === 0) { - this.setState({ - value, - errorMessage: - }); - } else { - this.setState({ value, errorMessage: "" }); - } - } - - handlePasswordValidation(event) { - const { value } = event.target; - if (value.length > PASSWORD_MAX_LENGTH) { - this.setState({ - value, - errorMessage: ( - - ) - }); - } else if (value.length < PASSWORD_MIN_LENGTH) { - this.setState({ - value, - errorMessage: ( - - ) - }); - } else { - this.setState({ value, errorMessage: "" }); - } - } - - handlePasswordConfirmationValidation(event) { - const { value } = event.target; - if (value !== this.state.value) { - this.setState({ - password_confirmation: value, - errorMessage: ( - - ) - }); - } else { - this.setState({ password_confirmation: value, errorMessage: "" }); + newVal = event.target.value; } + const newState = update(this.state, { + value: { $set: newVal } + }); + this.setState(newState); } handleCurrentPassword(event) { - const { value } = event.target; - if (value.length > PASSWORD_MAX_LENGTH) { - this.setState({ - current_password: value, - errorMessage: ( - - ) - }); - } else if (value.length < PASSWORD_MIN_LENGTH) { - this.setState({ - current_password: value, - errorMessage: ( - - ) - }); - } else { - this.setState({ current_password: value, errorMessage: "" }); - } + const newState = update(this.state, { + current_password: { $set: event.target.value } + }); + this.setState(newState); + } + + handlePasswordConfirmation(event) { + const newState = update(this.state, { + password_confirmation: { $set: event.target.value } + }); + this.setState(newState); } handleSubmit(event) { @@ -292,7 +140,7 @@ class InputEnabled extends Component { } }) .catch(({ response }) => { - this.setState({ errorMessage: response.data.message.toString() }); + this.form.setErrors(response.data.details); }); } @@ -301,108 +149,154 @@ class InputEnabled extends Component { if (type === "email") { return ( -
- - + + + + -
+ + ); } return ""; } inputField() { - const { inputType } = this.props; + const { inputType, dataField } = this.props; + + let validatorsOnChange = []; + if (dataField === "full_name") { + validatorsOnChange = [textBlankValidator, nameMaxLengthValidator]; + } else if (dataField === "initials") { + validatorsOnChange = [textBlankValidator, userInitialsMaxLengthValidator]; + } else if (dataField === "email") { + validatorsOnChange = [emailValidator]; + } + if (inputType === "password") { return (
- - - - - - - - - - + + + + + + + + + + + + + + + + + + + + +
); } if (inputType === "file") { return ( - + + + + ); } return ( - + + + + ); } render() { return ( - -
- -

-   - -

- {this.props.labelValue !== "none" && ( - - - - )} - {this.inputField()} - {this.confirmationField()} - {this.state.errorMessage} - - - - - -
+ + { this.form = f; }} + > +

+   + +

+ {this.props.labelValue !== "none" && ( + + + + )} + {this.inputField()} + {this.confirmationField()} + + + + + + + ); } diff --git a/app/services/client_api/user_service.rb b/app/services/client_api/user_service.rb deleted file mode 100644 index 15d87f27f..000000000 --- a/app/services/client_api/user_service.rb +++ /dev/null @@ -1,28 +0,0 @@ -module ClientApi - class UserService < BaseService - def update_user! - error = I18n.t('client_api.user.password_invalid') - raise CustomUserError, error unless check_current_password - @params.delete(:current_password) # removes unneeded element - @current_user.update(@params) - end - - private - - def check_current_password - return true unless @params[:email] || @params[:password] - pass_blank_err = I18n.t('client_api.user.blank_password_error') - pass_match_err = I18n.t('client_api.user.passwords_dont_match') - current_password = @params[:current_password] - raise CustomUserError, pass_blank_err unless current_password - raise CustomUserError, pass_match_err unless check_password_confirmation - @current_user.valid_password? current_password - end - - def check_password_confirmation - return true if @params[:email] - @params[:password] == @params[:password_confirmation] - end - end - CustomUserError = Class.new(StandardError) -end diff --git a/app/services/client_api/users/update_service.rb b/app/services/client_api/users/update_service.rb new file mode 100644 index 000000000..161976f4b --- /dev/null +++ b/app/services/client_api/users/update_service.rb @@ -0,0 +1,53 @@ +module ClientApi + module Users + class UpdateService < BaseService + attr_accessor :user + + def execute + @user = @current_user + + if current_password_valid? && + password_confirmation_valid? && + @user.update(@params.except(:current_password)) + success + else + error(@user.errors.full_messages.uniq.join('. ')) + end + end + + private + + def current_password_valid? + # Only check for current_password when updating + # email or password + return true unless @params[:email] || @params[:password] + + if @user.valid_password?(@params[:current_password]) + return true + else + @user.errors.add( + :current_password, + I18n.t('client_api.user.current_password_invalid') + ) + return false + end + end + + def password_confirmation_valid? + # Only check for password_confirmation when + # updating password + return true unless @params[:password] + + if @params[:password] == @params[:password_confirmation] + return true + else + @user.errors.add( + :password_confirmation, + I18n.t('client_api.user.password_confirmation_not_match') + ) + return false + end + end + end + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 533c8df61..d1ac3eff6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1822,9 +1822,7 @@ en: leave_team_error: "An error occured." leave_flash: "Successfuly left team %{team}." user: - blank_password_error: "Password can't be blank!" - passwords_dont_match: "Passwords don't match" - password_invalid: "Password is invalid!" - avatar_too_big: "Avatar file size must be less than 0.2 MB" + current_password_invalid: "incorrect password" + password_confirmation_not_match: "doesn't match" invite_users: permission_error: "You don't have permission to invite additional users to team. Contact its administrator/s." diff --git a/features/settings_page/profile.feature b/features/settings_page/profile.feature index 6ba58a9b2..e8c470614 100644 --- a/features/settings_page/profile.feature +++ b/features/settings_page/profile.feature @@ -103,7 +103,7 @@ Scenario: Unsuccessful Password Change, current password is invalid And I fill in "mypassword5678" in New password field And I fill in "mypassword5678" in New password confirmation field Then I click "Update" button - And I should see "Password is invalid!" + And I should see "incorrect password" @javascript Scenario: Successful Password Change diff --git a/spec/services/client_api/user_service_spec.rb b/spec/services/client_api/user_service_spec.rb deleted file mode 100644 index 2cc665785..000000000 --- a/spec/services/client_api/user_service_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -require 'rails_helper' - -describe ClientApi::UserService do - let(:user) do - create :user, - full_name: 'User One', - initials: 'UO', - email: 'user@happy.com', - password: 'asdf1234', - password_confirmation: 'asdf1234' - end - - describe '#update_user!' do - it 'should update user email if the password is correct' do - email = 'new_user@happy.com' - params = { email: email, current_password: 'asdf1234' } - user_service = ClientApi::UserService.new(current_user: user, - params: params) - user_service.update_user! - expect(user.email).to eq(email) - end - - it 'should raise CustomUserError error if the password is not correct' do - email = 'new_user@happy.com' - params = { email: email, current_password: 'banana' } - user_service = ClientApi::UserService.new(current_user: user, - params: params) - expect { - user_service.update_user! - }.to raise_error(ClientApi::CustomUserError) - end - - it 'should update initials and full name without password confirmation' do - full_name = 'Happy User' - initials = 'HU' - user_service = ClientApi::UserService.new( - current_user: user, - params: { full_name: full_name, initials: initials } - ) - user_service.update_user! - expect(user.full_name).to eq(full_name) - expect(user.initials).to eq(initials) - end - - it 'should raise an error if current password not present' do - user_service = ClientApi::UserService.new( - current_user: user, - params: { password: 'hello1234', password_confirmation: 'hello1234' } - ) - expect { - user_service.update_user! - }.to raise_error(ClientApi::CustomUserError) - end - - it 'should raise an error if password_confirmation don\'t match' do - user_service = ClientApi::UserService.new( - current_user: user, - params: { password: 'hello1234', - password_confirmation: 'hello1234567890', - current_password: 'asdf1234' } - ) - - expect { - user_service.update_user! - }.to raise_error(ClientApi::CustomUserError, 'Passwords don\'t match') - end - - it 'should update the password' do - new_password = 'hello1234' - user_service = ClientApi::UserService.new( - current_user: user, - params: { password: new_password, - password_confirmation: new_password, - current_password: 'asdf1234' } - ) - user_service.update_user! - expect(user.valid_password?(new_password)).to be(true) - end - end -end diff --git a/spec/services/client_api/users/update_service_spec.rb b/spec/services/client_api/users/update_service_spec.rb new file mode 100644 index 000000000..81998a9e0 --- /dev/null +++ b/spec/services/client_api/users/update_service_spec.rb @@ -0,0 +1,79 @@ +require 'rails_helper' + +include ClientApi::Users + +describe ClientApi::Users::UpdateService do + let(:user) do + create :user, + full_name: 'User One', + initials: 'UO', + email: 'user@happy.com', + password: 'asdf1234', + password_confirmation: 'asdf1234' + end + + it 'should update user email if the password is correct' do + email = 'new_user@happy.com' + params = { email: email, current_password: 'asdf1234' } + service = UpdateService.new(current_user: user, + params: params) + result = service.execute + expect(result[:status]).to eq :success + expect(user.email).to eq(email) + end + + it 'should raise CustomUserError error if the password is not correct' do + email = 'new_user@happy.com' + params = { email: email, current_password: 'banana' } + service = UpdateService.new(current_user: user, + params: params) + result = service.execute + expect(result[:status]).to eq :error + end + + it 'should update initials and full name without password confirmation' do + full_name = 'Happy User' + initials = 'HU' + service = UpdateService.new( + current_user: user, + params: { full_name: full_name, initials: initials } + ) + result = service.execute + expect(result[:status]).to eq :success + expect(user.full_name).to eq(full_name) + expect(user.initials).to eq(initials) + end + + it 'should raise an error if current password not present' do + service = UpdateService.new( + current_user: user, + params: { password: 'hello1234', password_confirmation: 'hello1234' } + ) + result = service.execute + expect(result[:status]).to eq :error + end + + it 'should raise an error if password_confirmation don\'t match' do + service = UpdateService.new( + current_user: user, + params: { password: 'hello1234', + password_confirmation: 'hello1234567890', + current_password: 'asdf1234' } + ) + result = service.execute + expect(result[:status]).to eq :error + end + + it 'should update the password' do + new_password = 'hello1234' + service = UpdateService.new( + current_user: user, + params: { password: new_password, + password_confirmation: new_password, + current_password: 'asdf1234' } + ) + result = service.execute + expect(result[:status]).to eq :success + expect(user.valid_password?(new_password)).to be(true) + end +end From 823ed7589823e752cbfa8f66104199f5c9bc22e8 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Tue, 24 Oct 2017 09:40:55 +0200 Subject: [PATCH 4/8] Add file validators, refactor a bit --- .../components/ValidatedFormControl.jsx | 4 +- .../components/validation/validators/file.js | 45 +++++++++++++++++ .../{text_validators.js => text.js} | 49 +++++++++++-------- .../src/config/constants/numeric.js | 1 + .../src/config/constants/strings.js | 1 + app/javascript/src/config/locales/messages.js | 6 ++- .../profile/components/InputEnabled.jsx | 8 ++- .../components/UpdateTeamDescriptionModal.jsx | 2 +- .../team/components/UpdateTeamNameModal.jsx | 2 +- .../SettingsPage/scenes/teams/new/index.jsx | 2 +- 10 files changed, 92 insertions(+), 28 deletions(-) create mode 100644 app/javascript/src/components/validation/validators/file.js rename app/javascript/src/components/validation/validators/{text_validators.js => text.js} (63%) diff --git a/app/javascript/src/components/validation/components/ValidatedFormControl.jsx b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx index 6297b4ce7..6d1be3771 100644 --- a/app/javascript/src/components/validation/components/ValidatedFormControl.jsx +++ b/app/javascript/src/components/validation/components/ValidatedFormControl.jsx @@ -13,7 +13,7 @@ class ValidatedFormControl extends Component { handleChange(e) { const tag = this.props.tag; const messageIds = this.props.messageIds; - const value = e.target.value; + const target = e.target; // Pass-through "original" onChange if (_.has(this.props, "onChange") && this.props.onChange !== undefined) { @@ -23,7 +23,7 @@ class ValidatedFormControl extends Component { // Validate the field let errors = []; this.props.validatorsOnChange.forEach((validator) => { - errors = errors.concat(validator(value, messageIds)); + errors = errors.concat(validator(target, messageIds)); }); this.context.setErrorsForTag(tag, errors); } diff --git a/app/javascript/src/components/validation/validators/file.js b/app/javascript/src/components/validation/validators/file.js new file mode 100644 index 000000000..b2371d055 --- /dev/null +++ b/app/javascript/src/components/validation/validators/file.js @@ -0,0 +1,45 @@ +import _ from "lodash"; +import { AVATAR_MAX_SIZE_MB } from "../../../config/constants/numeric"; +import { AVATAR_VALID_EXTENSIONS } from "../../../config/constants/strings"; + +export const avatarExtensionValidator = (target, messageIds = {}) => { + const messageId = _.has(messageIds, "invalid_file_extension") ? + messageIds.invalid_file_extension : + "validators.file.invalid_file_extension"; + + const filePath = target.value; + const ext = filePath + .substring(filePath.lastIndexOf(".") + 1) + .toLowerCase(); + const validExtsString = AVATAR_VALID_EXTENSIONS + .map(val => `.${val}`) + .join(", "); + + if (!_.includes(AVATAR_VALID_EXTENSIONS, ext)) { + return [{ + intl: true, + messageId, + values: { valid_extensions: validExtsString } + }]; + } + return []; +} + +export const avatarSizeValidator = (target, messageIds = {}) => { + const messageId = _.has(messageIds, "file_too_large") ? + messageIds.file_too_large : + "validators.file.file_too_large"; + const maxSizeKb = AVATAR_MAX_SIZE_MB * 1024; + + if (target.files && target.files[0]) { + const fileSize = target.files[0].size / 1024; // size in KB + if (fileSize > maxSizeKb) { + return [{ + intl: true, + messageId, + values: { max_size: AVATAR_MAX_SIZE_MB } + }]; + } + } + return []; +} \ No newline at end of file diff --git a/app/javascript/src/components/validation/validators/text_validators.js b/app/javascript/src/components/validation/validators/text.js similarity index 63% rename from app/javascript/src/components/validation/validators/text_validators.js rename to app/javascript/src/components/validation/validators/text.js index d2ad5233f..2ad97cb49 100644 --- a/app/javascript/src/components/validation/validators/text_validators.js +++ b/app/javascript/src/components/validation/validators/text.js @@ -9,10 +9,11 @@ import { } from "../../../config/constants/numeric"; import { EMAIL_REGEX } from "../../../config/constants/strings"; -export const nameMinLengthValidator = (value, messageIds = {}) => { +export const nameMinLengthValidator = (target, messageIds = {}) => { const messageId = _.has(messageIds, "text_too_short") ? messageIds.text_too_short : - "validators.text_validators.text_too_short"; + "validators.text.text_too_short"; + const value = target.value; if (value.length < NAME_MIN_LENGTH) { return [{ @@ -24,10 +25,11 @@ export const nameMinLengthValidator = (value, messageIds = {}) => { return []; }; -export const nameMaxLengthValidator = (value, messageIds = {}) => { - const messageId = _.has(messageIds, "text_too_long") ? - messageIds.text_too_long : - "validators.text_validators.text_too_long"; +export const nameMaxLengthValidator = (target, messageIds = {}) => { + const messageId = _.has(messageIds, "text_too_long") ? + messageIds.text_too_long : + "validators.text.text_too_long"; + const value = target.value; if (value.length > NAME_MAX_LENGTH) { return [{ @@ -39,18 +41,19 @@ export const nameMaxLengthValidator = (value, messageIds = {}) => { return []; }; -export const nameLengthValidator = (value, messageIds = {}) => { - const res = nameMinLengthValidator(value, messageIds); +export const nameLengthValidator = (target, messageIds = {}) => { + const res = nameMinLengthValidator(target, messageIds); if (res.length > 0) { return res; } - return nameMaxLengthValidator(value, messageIds); + return nameMaxLengthValidator(target, messageIds); }; -export const textBlankValidator = (value, messageIds = {}) => { +export const textBlankValidator = (target, messageIds = {}) => { const messageId = _.has(messageIds, "text_blank") ? messageIds.text_blank : - "validators.text_validators.text_blank"; + "validators.text.text_blank"; + const value = target.value; if (value.length === 0) { return [{ @@ -61,10 +64,11 @@ export const textBlankValidator = (value, messageIds = {}) => { return []; } -export const textMaxLengthValidator = (value, messageIds = {}) => { +export const textMaxLengthValidator = (target, messageIds = {}) => { const messageId = _.has(messageIds, "text_too_long") ? messageIds.text_too_long : - "validators.text_validators.text_too_long"; + "validators.text.text_too_long"; + const value = target.value; if (value.length > TEXT_MAX_LENGTH) { return [{ @@ -76,13 +80,14 @@ export const textMaxLengthValidator = (value, messageIds = {}) => { return []; }; -export const passwordLengthValidator = (value, messageIds = {}) => { +export const passwordLengthValidator = (target, messageIds = {}) => { const messageIdTooShort = _.has(messageIds, "text_too_short") ? messageIds.text_too_short : - "validators.text_validators.text_too_short"; + "validators.text.text_too_short"; const messageIdTooLong = _.has(messageIds, "text_too_long") ? messageIds.text_too_long : - "validators.text_validators.text_too_long"; + "validators.text.text_too_long"; + const value = target.value; if (value.length < PASSWORD_MIN_LENGTH) { return [{ @@ -100,10 +105,11 @@ export const passwordLengthValidator = (value, messageIds = {}) => { return []; }; -export const userInitialsMaxLengthValidator = (value, messageIds = {}) => { +export const userInitialsMaxLengthValidator = (target, messageIds = {}) => { const messageId = _.has(messageIds, "text_too_long") ? messageIds.text_too_long : - "validators.text_validators.text_too_long"; + "validators.text.text_too_long"; + const value = target.value; if (value.length > USER_INITIALS_MAX_LENGTH) { return [{ @@ -115,15 +121,16 @@ export const userInitialsMaxLengthValidator = (value, messageIds = {}) => { return []; }; -export const emailValidator = (value, messageIds = {}) => { - const res = textBlankValidator(value, messageIds); +export const emailValidator = (target, messageIds = {}) => { + const res = textBlankValidator(target, messageIds); if (res.length > 0) { return res; } const messageId = _.has(messageIds, "invalid_email") ? messageIds.invalid_email : - "validators.text_validators.invalid_email"; + "validators.text.invalid_email"; + const value = target.value; if (!EMAIL_REGEX.test(value)) { return [{ intl: true, messageId }]; diff --git a/app/javascript/src/config/constants/numeric.js b/app/javascript/src/config/constants/numeric.js index 945cfa6e9..b51e920d0 100644 --- a/app/javascript/src/config/constants/numeric.js +++ b/app/javascript/src/config/constants/numeric.js @@ -6,3 +6,4 @@ export const PASSWORD_MAX_LENGTH = 72; export const NAME_MAX_LENGTH = 255; export const TEXT_MAX_LENGTH = 10000; export const INVITE_USERS_LIMIT = 20; +export const AVATAR_MAX_SIZE_MB = 0.2; \ No newline at end of file diff --git a/app/javascript/src/config/constants/strings.js b/app/javascript/src/config/constants/strings.js index f0276bb21..cd941e8a6 100644 --- a/app/javascript/src/config/constants/strings.js +++ b/app/javascript/src/config/constants/strings.js @@ -2,3 +2,4 @@ export const ASSIGNMENT_NOTIFICATION = "ASSIGNMENT"; export const RECENT_NOTIFICATION = "RECENT_NOTIFICATION"; export const SYSTEM_NOTIFICATION = "SYSTEM_NOTIFICATION"; export const EMAIL_REGEX = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/; +export const AVATAR_VALID_EXTENSIONS = ["jpg", "jpeg", "png", "gif"]; \ No newline at end of file diff --git a/app/javascript/src/config/locales/messages.js b/app/javascript/src/config/locales/messages.js index 3cbbdeb25..5f480186c 100644 --- a/app/javascript/src/config/locales/messages.js +++ b/app/javascript/src/config/locales/messages.js @@ -17,11 +17,15 @@ export default { new_team_page: "SciNote | Settings | Teams | New" }, validators: { - text_validators: { + text: { text_too_short: "is too short (minimum is {min_length} characters)", text_too_long: "is too long (maximum is {max_length} characters)", text_blank: "can't be blank", invalid_email: "invalid email" + }, + file: { + invalid_file_extension: "invalid file extension (valid extensions are {valid_extensions})", + file_too_large: "file too large (maximum size is {max_size} MB)" } }, error_messages: { diff --git a/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx b/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx index 7efacc543..aebda3857 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/profile/components/InputEnabled.jsx @@ -32,7 +32,11 @@ import { passwordLengthValidator, userInitialsMaxLengthValidator, emailValidator -} from "../../../../../components/validation/validators/text_validators"; +} from "../../../../../components/validation/validators/text"; +import { + avatarExtensionValidator, + avatarSizeValidator +} from "../../../../../components/validation/validators/file"; const StyledInputEnabled = styled.div` border: 1px solid ${BORDER_LIGHT_COLOR}; @@ -179,6 +183,8 @@ class InputEnabled extends Component { validatorsOnChange = [textBlankValidator, userInitialsMaxLengthValidator]; } else if (dataField === "email") { validatorsOnChange = [emailValidator]; + } else if (dataField === "avatar") { + validatorsOnChange = [avatarExtensionValidator, avatarSizeValidator]; } if (inputType === "password") { diff --git a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx index fc54f563b..32735d75e 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamDescriptionModal.jsx @@ -17,7 +17,7 @@ import { } from "../../../../../components/validation"; import { textMaxLengthValidator -} from "../../../../../components/validation/validators/text_validators"; +} from "../../../../../components/validation/validators/text"; import { TEAM_UPDATE_PATH } from "../../../../../config/api_endpoints"; diff --git a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx index 2bed07904..6e18cfb24 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/team/components/UpdateTeamNameModal.jsx @@ -17,7 +17,7 @@ import { } from "../../../../../components/validation"; import { nameLengthValidator -} from "../../../../../components/validation/validators/text_validators"; +} from "../../../../../components/validation/validators/text"; import { TEAM_UPDATE_PATH } from "../../../../../config/api_endpoints"; diff --git a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx index 64306d18f..41e39ab28 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx @@ -33,7 +33,7 @@ import { nameLengthValidator, textMaxLengthValidator -} from "../../../../../components/validation/validators/text_validators"; +} from "../../../../../components/validation/validators/text"; import { BORDER_LIGHT_COLOR } from "../../../../../config/constants/colors"; From 998519066c35e6c19e9bd0d3766ca8eeb986f75d Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Tue, 24 Oct 2017 10:00:10 +0200 Subject: [PATCH 5/8] Remove unneeded localized Strings --- app/javascript/src/config/locales/messages.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/javascript/src/config/locales/messages.js b/app/javascript/src/config/locales/messages.js index 5f480186c..4f6cd1389 100644 --- a/app/javascript/src/config/locales/messages.js +++ b/app/javascript/src/config/locales/messages.js @@ -28,13 +28,6 @@ export default { file_too_large: "file too large (maximum size is {max_size} MB)" } }, - error_messages: { - text_too_short: "is too short (minimum is {min_length} characters)", - text_too_long: "is too long (maximum is {max_length} characters)", - cant_be_blank: "can't be blank", - invalid_email: "invalid email", - passwords_dont_match: "Passwords don't match" - }, navbar: { page_title: "sciNote", home_label: "Home", From 0eea1e3def15d08b5a82808e48906c898c3551fb Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Tue, 24 Oct 2017 13:49:33 +0200 Subject: [PATCH 6/8] Fixes per @ZmagoD's request --- .../components/ValidatedErrorHelpBlock.jsx | 13 +++++-------- .../validation/components/ValidatedFormGroup.jsx | 15 +++------------ .../SettingsPage/scenes/teams/new/index.jsx | 1 - features/settings_page/profile.feature | 6 +++--- 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx index 2e9b1afa8..21576e339 100644 --- a/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx +++ b/app/javascript/src/components/validation/components/ValidatedErrorHelpBlock.jsx @@ -26,12 +26,6 @@ class ValidatedErrorHelpBlock extends Component { return {error.message}; } - constructor(props) { - super(props); - - this.cleanProps = this.cleanProps.bind(this); - } - cleanProps() { // Remove additional props from the props const { tag, ...cleanProps } = this.props; @@ -39,9 +33,12 @@ class ValidatedErrorHelpBlock extends Component { } render() { - const errors = this.context.errors(this.props.tag) || []; + // Remove additional props from the props + const { tag, ...cleanProps } = this.props; + + const errors = this.context.errors(tag) || []; return ( - + {errors.map((error) => ValidatedErrorHelpBlock.renderErrorMessage(error))} ); diff --git a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx index e8a5e11d8..df31040f0 100644 --- a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx +++ b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx @@ -3,20 +3,11 @@ import { FormGroup } from "react-bootstrap"; import PropTypes from "prop-types"; class ValidatedFormGroup extends Component { - constructor(props) { - super(props); - - this.cleanProps = this.cleanProps.bind(this); - } - - cleanProps() { + render() { // Remove additional props from the props const { tag, ...cleanProps } = this.props; - return cleanProps; - } - render() { - const hasError = this.context.hasErrorForTag(this.props.tag); + const hasError = this.context.hasErrorForTag(tag); const formGroupClass = `form-group ${hasError ? " has-error" : ""}`; const validationState = hasError ? "error" : null; @@ -24,7 +15,7 @@ class ValidatedFormGroup extends Component { ); } diff --git a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx index 41e39ab28..6bce95f43 100644 --- a/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx +++ b/app/javascript/src/scenes/SettingsPage/scenes/teams/new/index.jsx @@ -20,7 +20,6 @@ import { SETTINGS_TEAMS_ROUTE, SETTINGS_TEAM_ROUTE } from "../../../../../config/routes"; -import { TEAMS_NEW_PATH } from "../../../../../config/api_endpoints"; import { getTeamsList } from "../../../../../components/actions/TeamsActions"; import { ValidatedForm, diff --git a/features/settings_page/profile.feature b/features/settings_page/profile.feature index e8c470614..88d2ad575 100644 --- a/features/settings_page/profile.feature +++ b/features/settings_page/profile.feature @@ -24,7 +24,7 @@ Scenario: Unsuccessful avatar image upload, file is too big Then I click on image within ".avatar-container" element And I attach a "Moon.png" file to "user_avatar_input" field Then I click "Update" button - And I should see "Avatar file size must be less than 0.2 MB" error message under "user_avatar_input" field + And I should see "file too large (maximum size is 0.2 MB)" error message under "user_avatar_input" field @javascript Scenario: Unsuccessful avatar image upload, file is invalid @@ -32,7 +32,7 @@ Scenario: Unsuccessful avatar image upload, file is invalid Then I click on image within ".avatar-container" element And I attach a "File.txt" file to "user_avatar_input" field Then I click "Update" button - And I should see "Avatar content type is invalid" error message under "user_avatar_input" field + And I should see "invalid file extension" error message under "user_avatar_input" field @javascript Scenario: Successful upload avatar image @@ -93,7 +93,7 @@ Scenario: Unsuccessful Password Change, passwords does not match And I fill in "mypassword5678" in New password field And I fill in "mypassword56788" in New password confirmation field Then I click "Update" button - And I should see "Passwords don't match" + And I should see "doesn't match" @javascript Scenario: Unsuccessful Password Change, current password is invalid From cbe3e6740243e7c8240916108be598812ce00f97 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Tue, 24 Oct 2017 13:57:33 +0200 Subject: [PATCH 7/8] Codestyle refactor --- .../components/ValidatedFormGroup.jsx | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx index df31040f0..8f0059990 100644 --- a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx +++ b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx @@ -2,24 +2,22 @@ import React, { Component } from "react"; import { FormGroup } from "react-bootstrap"; import PropTypes from "prop-types"; -class ValidatedFormGroup extends Component { - render() { - // Remove additional props from the props - const { tag, ...cleanProps } = this.props; +const ValidatedFormGroup = (props, context) => { + // Remove additional props from the props + const { tag, ...cleanProps } = props; - const hasError = this.context.hasErrorForTag(tag); - const formGroupClass = `form-group ${hasError ? " has-error" : ""}`; - const validationState = hasError ? "error" : null; + const hasError = context.hasErrorForTag(tag); + const formGroupClass = `form-group ${hasError ? " has-error" : ""}`; + const validationState = hasError ? "error" : null; - return ( - - ); - } -} + return ( + + ); +}; ValidatedFormGroup.propTypes = { tag: PropTypes.string.isRequired From a60e4c28bb5abcb6e8f7c30976e5058c710b0a18 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Tue, 24 Oct 2017 13:58:59 +0200 Subject: [PATCH 8/8] Hound is love, Hound is life --- .../src/components/validation/components/ValidatedFormGroup.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx index 8f0059990..0adb2c889 100644 --- a/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx +++ b/app/javascript/src/components/validation/components/ValidatedFormGroup.jsx @@ -1,4 +1,4 @@ -import React, { Component } from "react"; +import React from "react"; import { FormGroup } from "react-bootstrap"; import PropTypes from "prop-types";