diff --git a/src/components/presentation-review-form.js b/src/components/presentation-review-form.js index 213f0f9..a26dfa8 100644 --- a/src/components/presentation-review-form.js +++ b/src/components/presentation-review-form.js @@ -16,7 +16,9 @@ import 'awesome-bootstrap-checkbox/awesome-bootstrap-checkbox.css' import SubmitButtons from "./presentation-submit-buttons"; import { RawHTML } from 'openstack-uicore-foundation/lib/components' import T from "i18n-react/dist/i18n-react"; +import Swal from "sweetalert2"; import {getMarketingValue} from "./marketing-setting"; +import { validateSpeakerCount } from './speaker-limits'; class PresentationReviewForm extends React.Component { constructor(props) { @@ -42,6 +44,20 @@ class PresentationReviewForm extends React.Component { handleSubmit(ev) { ev.preventDefault(); + const { entity, selectionPlanSettings } = this.props; + const speakerValidation = validateSpeakerCount(entity); + + if (!speakerValidation.valid) { + const presentation = selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation").toLowerCase(); + const speaker = (selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.speaker")).toLowerCase(); + const speakers = (selectionPlanSettings?.CFP_SPEAKERS_PLURAL_LABEL || T.translate("edit_presentation.speakers")).toLowerCase(); + const { errorField, min, max, excess } = speakerValidation; + const translationParams = { presentation, speaker, speakers, max, min, excess }; + + Swal.fire("Validation error", T.translate(`edit_presentation.errors.${errorField}`, translationParams), "warning"); + return; + } + this.props.onSubmit(this.props.entity); } diff --git a/src/components/presentation-speakers-form.js b/src/components/presentation-speakers-form.js index cbbacec..3f26c98 100644 --- a/src/components/presentation-speakers-form.js +++ b/src/components/presentation-speakers-form.js @@ -18,7 +18,7 @@ import { Exclusive, Dropdown } from 'openstack-uicore-foundation/lib/components' import T from "i18n-react/dist/i18n-react"; import CPFSpeakerInput from './inputs/speaker-input' import Swal from "sweetalert2"; -import {getMarketingValue} from "./marketing-setting"; +import { getSpeakerLimits, validateSpeakerCount } from './speaker-limits'; class PresentationSpeakersForm extends React.Component { constructor(props) { @@ -27,8 +27,8 @@ class PresentationSpeakersForm extends React.Component { this.state = { speaker: {}, speakerInput: null, - entity: {...props.entity}, - currentSpeakerType : null + entity: { ...props.entity }, + currentSpeakerType: null }; this.handleSubmit = this.handleSubmit.bind(this); @@ -40,31 +40,37 @@ class PresentationSpeakersForm extends React.Component { this.handleAddSpeaker = this.handleAddSpeaker.bind(this); } - handleChangeSpeakerType(ev){ - let {value, id} = ev.target; + handleChangeSpeakerType(ev) { + let { value, id } = ev.target; id = id.toString(); - this.setState({...this.state, currentSpeakerType: value, error: null}); + this.setState({ ...this.state, currentSpeakerType: value, error: null }); } handleSubmit(ev) { - - const entity = {...this.props.entity}; - const { selectionPlanSettings } = this.props; ev.preventDefault(); + const { selectionPlanSettings, entity } = this.props; const validModerator = !entity.type.use_moderator || !entity.type.is_moderator_mandatory || entity.moderator; - const validSpeaker = !entity.type.use_speakers || !entity.type.are_speakers_mandatory || entity.speakers.length > 0; + const presentation = selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation").toLowerCase(); if (!validModerator) { - Swal.fire("Validation error", T.translate("edit_presentation.errors.add_moderator", - { presentation: selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation").toLowerCase()}), "warning"); - } else if (!validSpeaker) { - Swal.fire("Validation error", T.translate("edit_presentation.errors.add_speaker", - { presentation: selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation").toLowerCase(), - speaker: selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.speaker").toLowerCase()}), "warning"); - } else { - this.props.onSubmit(this.props.entity); + Swal.fire("Validation error", T.translate("edit_presentation.errors.add_moderator", { presentation }), "warning"); + return; } + + const speakerValidation = validateSpeakerCount(entity); + + if (!speakerValidation.valid) { + const speaker = (selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.speaker")).toLowerCase(); + const speakers = (selectionPlanSettings?.CFP_SPEAKERS_PLURAL_LABEL || T.translate("edit_presentation.speakers")).toLowerCase(); + const { errorField, min, max, excess } = speakerValidation; + const translationParams = { presentation, speaker, speakers, max, min, excess }; + + Swal.fire("Validation error", T.translate(`edit_presentation.errors.${errorField}`, translationParams), "warning"); + return; + } + + this.props.onSubmit(this.props.entity); } handleBack(ev) { @@ -73,12 +79,12 @@ class PresentationSpeakersForm extends React.Component { } handleChangeSpeaker(ev) { - let {value, id} = ev.target; - this.setState({speaker: value, speakerInput: value}); + let { value, id } = ev.target; + this.setState({ speaker: value, speakerInput: value }); } handleSpeakerClick(speakerId, speakerType, ev) { - let {history, entity, summit} = this.props; + let { history, entity, summit } = this.props; ev.preventDefault(); this.props.onSpeakerEdit(entity.selection_plan_id, entity.id, speakerId, speakerType); } @@ -94,24 +100,25 @@ class PresentationSpeakersForm extends React.Component { } handleEditSpeaker(speakerId, speakerType, ev) { - let {history, entity} = this.props; + let { history, entity } = this.props; ev.preventDefault(); this.props.onSpeakerEdit(entity.selection_plan_id, entity.id, speakerId, speakerType); } handleAddSpeaker(ev) { - const {speaker, currentSpeakerType} = this.state; - const {history, onAddSpeaker, onAddModerator, match} = this.props; + const { speaker, currentSpeakerType } = this.state; + const { history, onAddSpeaker, onAddModerator, match, selectionPlanSettings } = this.props; + const speakerLabel = selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.speaker").toLowerCase(); ev.preventDefault(); - if(!speaker){ + if (!speaker) { // speaker not set - this.setState({...this.state, error: T.translate("edit_presentation.errors.missing_speaker")}); + this.setState({ ...this.state, error: T.translate("edit_presentation.errors.missing_speaker", { speaker: speakerLabel }) }); return; } - if(!currentSpeakerType){ - this.setState({...this.state, error: T.translate("edit_presentation.errors.role")}); + if (!currentSpeakerType) { + this.setState({ ...this.state, error: T.translate("edit_presentation.errors.role") }); return; } @@ -122,7 +129,7 @@ class PresentationSpeakersForm extends React.Component { } else { onAddSpeaker(speaker); } - this.setState({...this.state, currentSpeakerType: null, speakerInput: null, error: null}); + this.setState({ ...this.state, currentSpeakerType: null, speakerInput: null, error: null }); return false; } @@ -134,41 +141,43 @@ class PresentationSpeakersForm extends React.Component { } // speaker not set - this.setState({...this.state, error: T.translate("edit_presentation.errors.missing_speaker")}); + this.setState({ ...this.state, error: T.translate("edit_presentation.errors.missing_speaker", { speaker: speakerLabel }) }); return false; } render() { - let {summit, selectionPlanSettings, entity, presentation, step} = this.props; - let {speakerInput, error, speaker} = this.state; - let eventType = summit.event_types.find(t => t.id == entity.type_id); - let canAddSpeakers = (eventType && eventType.max_speakers > entity.speakers.length); + let { summit, selectionPlanSettings, entity, presentation, step } = this.props; + let { speakerInput, error, speaker } = this.state; + let eventType = summit.event_types.find(t => t.id == entity.type_id); + let speakersCount = entity.speakers?.length ?? 0; + let { max: maxSpeakers } = getSpeakerLimits(entity.type); + let canAddSpeakers = (eventType && maxSpeakers > speakersCount); let canAddModerator = (eventType && eventType.max_moderators && !entity.moderator); let speakerTypes = []; - if(canAddSpeakers){ - speakerTypes.push({value:'speaker', label: selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.labels.speaker")}); + if (canAddSpeakers) { + speakerTypes.push({ value: 'speaker', label: selectionPlanSettings?.CFP_SPEAKERS_SINGULAR_LABEL || T.translate("edit_presentation.labels.speaker") }); } - if(canAddModerator){ - speakerTypes.push({value:'moderator', label: T.translate("edit_presentation.labels.moderator")}); + if (canAddModerator) { + speakerTypes.push({ value: 'moderator', label: T.translate("edit_presentation.labels.moderator") }); } return (
-

{T.translate("edit_presentation.speaker_included", - { presentation: selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation")})}

+

{T.translate("edit_presentation.speaker_included", + { presentation: selectionPlanSettings?.CFP_PRESENTATIONS_SINGULAR_LABEL || T.translate("edit_presentation.presentation") })}

- + -
+
{entity.moderator &&
@@ -186,7 +195,7 @@ class PresentationSpeakersForm extends React.Component {
} - {entity.speakers.map(s => ( + {entity.speakers?.map(s => (
@@ -208,54 +217,55 @@ class PresentationSpeakersForm extends React.Component {
- + {speakerTypes.length > 0 && -
-
-
-
- - -
-
- - -
-
- +
+
+
+
+ + +
+
+ + +
+
+ +
+ {error && +

{error}

} + {speaker && isNaN(speaker.id) && (speaker.value) && +

* Please select a Role and click "Add".

}
- {error && -

{error}

} - {speaker && isNaN(speaker.id) && (speaker.value) && -

* Please select a Role and click "Add".

} -
} {!canAddSpeakers && !canAddModerator && -

{T.translate("edit_presentation.max_speakers")}

+

{T.translate("edit_presentation.max_speakers")}

} @@ -264,7 +274,7 @@ class PresentationSpeakersForm extends React.Component {
-
+
diff --git a/src/components/speaker-limits/__tests__/speaker-limits.test.js b/src/components/speaker-limits/__tests__/speaker-limits.test.js new file mode 100644 index 0000000..d74fa87 --- /dev/null +++ b/src/components/speaker-limits/__tests__/speaker-limits.test.js @@ -0,0 +1,99 @@ +/** + * Copyright 2017 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import { getSpeakerLimits, getSpeakerCountErrorField, validateSpeakerCount } from '..'; + +describe('getSpeakerLimits', () => { + it('returns zero/zero when no event type is available yet', () => { + expect(getSpeakerLimits(null)).toEqual({ min: 0, max: 0 }); + expect(getSpeakerLimits(undefined)).toEqual({ min: 0, max: 0 }); + }); + + it('defaults to zero/zero when the event type does not use speakers at all', () => { + expect(getSpeakerLimits({ use_speakers: false, are_speakers_mandatory: false, min_speakers: 0, max_speakers: 0 })).toEqual({ min: 0, max: 0 }); + }); + + it('honors an explicit min_speakers of 0 even when speakers are mandatory', () => { + // min_speakers=0 is a real, distinct configuration from a positive minimum - + // the API always sends a concrete min_speakers, so 0 must pass through as-is. + expect(getSpeakerLimits({ are_speakers_mandatory: true, min_speakers: 0, max_speakers: 5 })).toEqual({ min: 0, max: 5 }); + }); + + it('honors an explicit finite range', () => { + expect(getSpeakerLimits({ use_speakers: true, min_speakers: 2, max_speakers: 5 })).toEqual({ min: 2, max: 5 }); + }); + + it('honors an explicit exact-count configuration (min equals max)', () => { + expect(getSpeakerLimits({ use_speakers: true, min_speakers: 3, max_speakers: 3 })).toEqual({ min: 3, max: 3 }); + }); + + it('clamps max_speakers up to min_speakers when the event type is misconfigured with max below min', () => { + expect(getSpeakerLimits({ use_speakers: true, min_speakers: 3, max_speakers: 1 })).toEqual({ min: 3, max: 3 }); + }); + + it('honors an explicit max_speakers of 0 when min is also 0', () => { + expect(getSpeakerLimits({ use_speakers: true, min_speakers: 0, max_speakers: 0 })).toEqual({ min: 0, max: 0 }); + }); +}); + +describe('getSpeakerCountErrorField', () => { + it('reports remove_speakers when the count exceeds the max, even for an exact-count requirement', () => { + // over-max must win over the min===max branch below - a submitter over + // the limit needs to be told to remove speakers, not to add the exact count. + expect(getSpeakerCountErrorField(5, 3, 3)).toBe('remove_speakers'); + }); + + it('reports add_only_one_speaker when exactly one speaker is required', () => { + expect(getSpeakerCountErrorField(0, 1, 1)).toBe('add_only_one_speaker'); + }); + + it('reports add_exact_number_of_speakers when an exact count greater than one is required', () => { + expect(getSpeakerCountErrorField(1, 3, 3)).toBe('add_exact_number_of_speakers'); + }); + + it('reports add_speakers for a bounded range with distinct min and max', () => { + expect(getSpeakerCountErrorField(0, 2, 5)).toBe('add_speakers'); + }); +}); + +describe('validateSpeakerCount', () => { + it('is valid when the event type does not use speakers at all, regardless of count', () => { + expect(validateSpeakerCount({ type: { use_speakers: false }, speakers: [] })).toEqual({ valid: true }); + }); + + it('is invalid when exactly one speaker is required and none were added', () => { + const result = validateSpeakerCount({ type: { use_speakers: true, min_speakers: 1, max_speakers: 1 }, speakers: [] }); + expect(result.valid).toBe(false); + expect(result.errorField).toBe('add_only_one_speaker'); + }); + + it('is valid once enough speakers were added to satisfy the minimum', () => { + const speakers = [{ id: 1 }, { id: 2 }]; + expect(validateSpeakerCount({ type: { use_speakers: true, min_speakers: 2, max_speakers: 5 }, speakers })).toEqual({ valid: true }); + }); + + it('is invalid, with the correct excess, when the count exceeds the max', () => { + // regression test for the Complete-action bypass: a speaker removed on the Speakers step + // can leave a presentation over/under limits without ever re-running this check there, so + // this same function must also gate the Review step's Complete action before it submits. + const speakers = [{ id: 1 }, { id: 2 }, { id: 3 }]; + const result = validateSpeakerCount({ type: { use_speakers: true, min_speakers: 1, max_speakers: 2 }, speakers }); + expect(result).toEqual({ valid: false, errorField: 'remove_speakers', min: 1, max: 2, excess: 1 }); + }); + + it('is invalid when the entity has no speakers array at all', () => { + const result = validateSpeakerCount({ type: { use_speakers: true, min_speakers: 1, max_speakers: 1 } }); + expect(result.valid).toBe(false); + expect(result.errorField).toBe('add_only_one_speaker'); + }); +}); diff --git a/src/components/speaker-limits/index.js b/src/components/speaker-limits/index.js new file mode 100644 index 0000000..48e6b4d --- /dev/null +++ b/src/components/speaker-limits/index.js @@ -0,0 +1,57 @@ +/** + * Copyright 2017 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +export const getSpeakerLimits = (type) => { + if (!type) return { min: 0, max: 0 }; + // min_speakers/max_speakers are non-nullable ints on the API's PresentationType + // (see PresentationTypeSerializer) - always present, never Infinity/unbounded. + const { min_speakers: min, max_speakers: max } = type; + // Protection against invalid configuration of max_speakers < min_speakers + return { min, max: max >= min ? max : min }; +}; + +export const getSpeakerCountErrorField = (speakersCount, minSpeakers, maxSpeakers) => { + if (speakersCount > maxSpeakers) return "remove_speakers"; + + switch (true) { + // There should be only one speaker + case (minSpeakers === maxSpeakers && maxSpeakers === 1): + return "add_only_one_speaker"; + // There should be exactly a number of speakers + case (minSpeakers === maxSpeakers && maxSpeakers !== 1): + return "add_exact_number_of_speakers"; + // The default error message when there is an upper limit and a minimum of speakers + default: + return "add_speakers"; + } +}; + +// Single source of truth for "is this presentation's speaker count valid" - used both +// by the Speakers step (on save) and the Review step (on final Complete), so the +// Complete action can't finalize a presentation the Speakers step would have rejected. +export const validateSpeakerCount = (entity) => { + if (!entity?.type?.use_speakers) return { valid: true }; + + const speakersCount = Array.isArray(entity.speakers) ? entity.speakers.length : 0; + const { min, max } = getSpeakerLimits(entity.type); + + if (speakersCount <= max && speakersCount >= min) return { valid: true }; + + return { + valid: false, + errorField: getSpeakerCountErrorField(speakersCount, min, max), + min, + max, + excess: speakersCount - max + }; +}; diff --git a/src/i18n/en.json b/src/i18n/en.json index bc37772..83438e1 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -205,8 +205,11 @@ "errors": { "role": "You must select a Role.", "add_moderator": "You need to add a moderator to the {presentation}.", - "add_speaker": "You need to add at least one {speaker} to the {presentation}.", - "missing_speaker": "You need to select a participant." + "add_speakers": "You need to add between {min} and {max} {speakers} to the {presentation}.", + "add_only_one_speaker": "You need to add one {speaker} to the {presentation}.", + "add_exact_number_of_speakers": "You need to add {min} {speakers} to the {presentation}.", + "missing_speaker": "You need to select a {speaker}.", + "remove_speakers": "You can have at most {max} {speakers} in the {presentation}. Please remove {excess}." } }, "edit_speaker": {