Skip to content

Commit

Permalink
Merge pull request #3597 from LiteFarmOrg/LF-4583-users-are-currently…
Browse files Browse the repository at this point in the history
…-able-to-create-a-duplicate-of-an-already-existing-lite-farm-animal-type-or-breed

Lf 4583 users are currently able to create a duplicate of an already existing lite farm animal type or breed
  • Loading branch information
antsgar authored Dec 12, 2024
2 parents 1a60bdb + a7a0a06 commit e27e81d
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 76 deletions.
9 changes: 0 additions & 9 deletions packages/api/src/controllers/baseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,6 @@ export default {
return await model.query(trx).eager(subModel);
},

/**
* Format transaltion key
* @param {String} key
* @returns {String} - Formatted key
*/
formatTranslationKey(key) {
return key.toUpperCase().trim().replaceAll(' ', '_');
},

/**
* To check if record is deleted or not
* @param {Object} trx - Transaction object
Expand Down
5 changes: 3 additions & 2 deletions packages/api/src/controllers/farmExpenseTypeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import baseController from '../controllers/baseController.js';
import ExpenseTypeModel from '../models/expenseTypeModel.js';
import FarmExpenseModel from '../models/farmExpenseModel.js';
import { transaction, Model } from 'objection';
import { formatTranslationKey } from '../util/util.js';

const farmExpenseTypeController = {
addFarmExpenseType() {
Expand All @@ -26,7 +27,7 @@ const farmExpenseTypeController = {
try {
const farm_id = req.headers.farm_id;
const data = req.body;
data.expense_translation_key = baseController.formatTranslationKey(data.expense_name);
data.expense_translation_key = formatTranslationKey(data.expense_name);
//prevent empty strings
data.custom_description = data.custom_description || null;

Expand Down Expand Up @@ -168,7 +169,7 @@ const farmExpenseTypeController = {
return res.status(409).send();
}

data.expense_translation_key = baseController.formatTranslationKey(data.expense_name);
data.expense_translation_key = formatTranslationKey(data.expense_name);
//prevent empty strings
data.custom_description = data.custom_description || null;

Expand Down
5 changes: 3 additions & 2 deletions packages/api/src/controllers/revenueTypeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import baseController from './baseController.js';
import RevenueTypeModel from '../models/revenueTypeModel.js';
import SaleModel from '../models/saleModel.js';
import { transaction, Model } from 'objection';
import { formatTranslationKey } from '../util/util.js';

const revenueTypeController = {
addType() {
Expand All @@ -26,7 +27,7 @@ const revenueTypeController = {
try {
const farm_id = req.headers.farm_id;
const data = req.body;
data.revenue_translation_key = baseController.formatTranslationKey(data.revenue_name);
data.revenue_translation_key = formatTranslationKey(data.revenue_name);
//prevent empty strings
data.custom_description = data.custom_description || null;

Expand Down Expand Up @@ -181,7 +182,7 @@ const revenueTypeController = {
return res.status(409).send();
}

data.revenue_translation_key = baseController.formatTranslationKey(data.revenue_name);
data.revenue_translation_key = formatTranslationKey(data.revenue_name);

const result = await baseController.patch(RevenueTypeModel, revenue_type_id, data, req, {
trx,
Expand Down
38 changes: 20 additions & 18 deletions packages/api/src/middleware/validation/checkAnimalOrBatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import AnimalUseModel from '../../models/animalUseModel.js';
import AnimalOriginModel from '../../models/animalOriginModel.js';
import AnimalIdentifierType from '../../models/animalIdentifierTypeModel.js';
import { ANIMAL_CREATE_LIMIT } from '../../util/animal.js';
import { compareUpperCaseTrim, upperCaseTrim } from '../../util/util.js';

const AnimalOrBatchModel = {
animal: AnimalModel,
Expand Down Expand Up @@ -393,30 +394,31 @@ const getRecordIfExists = async (animalOrBatch, animalOrBatchKey, farm_id) => {
.withGraphFetched(relations);
};

// Post loop checks
// Checks for duplicates - no migration was made for pre-existing duplicates on beta
// Currently the only endpoint checking case
const checkCustomTypeAndBreedConflicts = async (newTypesSet, newBreedsSet, farm_id, trx) => {
if (newTypesSet.size) {
const record = await CustomAnimalTypeModel.getTypesByFarmAndTypes(
farm_id,
[...newTypesSet],
trx,
);

if (record.length) {
throw customError('Animal type already exists', 409);
}
const customTypes = await CustomAnimalTypeModel.query(trx).where('farm_id', farm_id);
const formattedCustomTypes = [...customTypes].map((ct) => upperCaseTrim(ct.type));
newTypesSet.forEach((newType) => {
if (formattedCustomTypes.includes(upperCaseTrim(newType))) {
throw customError('Animal type already exists', 409);
}
});
}

if (newBreedsSet.size) {
const typeBreedPairs = [...newBreedsSet].map((breed) => breed.split('/'));
const record = await CustomAnimalBreedModel.getBreedsByFarmAndTypeBreedPairs(
farm_id,
typeBreedPairs,
trx,
);

if (record.length) {
throw customError('Animal breed already exists', 409);
const customBreeds = await CustomAnimalBreedModel.query(trx).where('farm_id', farm_id);
for (const newBreed of typeBreedPairs) {
const [typeColumn, typeId, breed] = newBreed;
if (
[...customBreeds].some(
(cb) => cb[typeColumn] === +typeId && compareUpperCaseTrim(cb.breed, breed),
)
) {
throw customError('Animal breed already exists', 409);
}
}
}
};
Expand Down
14 changes: 0 additions & 14 deletions packages/api/src/models/customAnimalBreedModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,6 @@ class CustomAnimalBreed extends baseModel {
additionalProperties: false,
};
}

static async getBreedsByFarmAndTypeBreedPairs(farm_id, typeBreeds, trx) {
const conditions = typeBreeds.map(([typeColumn, typeId, breed]) => {
return trx.raw(`(${typeColumn} = ? AND breed = ?)`, [typeId, breed]);
});
const data = await trx.raw(
`SELECT id
FROM
custom_animal_breed
WHERE farm_id = ? AND deleted is FALSE AND (${conditions.join(' OR ')});`,
[farm_id],
);
return data.rows;
}
}

export default CustomAnimalBreed;
11 changes: 0 additions & 11 deletions packages/api/src/models/customAnimalTypeModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,6 @@ class CustomAnimalType extends baseModel {
);
return data.rows;
}

static async getTypesByFarmAndTypes(farm_id, types, trx) {
const data = await trx.raw(
`SELECT id
FROM
custom_animal_type
WHERE farm_id = ? AND deleted is FALSE AND type IN (${types.map(() => '?').join(',')});`,
[farm_id, ...types],
);
return data.rows;
}
}

export default CustomAnimalType;
24 changes: 24 additions & 0 deletions packages/api/src/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,27 @@ export const checkAndTrimString = (input) => {
}
return input.trim();
};

/**
* Format translation key
* @param {String} key
* @returns {String} - Formatted key
*/
export const formatTranslationKey = (key) => {
return key.toUpperCase().trim().replaceAll(' ', '_');
};

export const upperCaseTrim = (a) => {
return a.toUpperCase().trim();
};

/**
* Check for duplicate or matching strings - (same as frontend util)
* TODO: consider localeCompare() or not caring about case sensitivity
* @param {String} a
* @param {String} b
* @returns {Boolean}
*/
export const compareUpperCaseTrim = (a, b) => {
return upperCaseTrim(a) === upperCaseTrim(b);
};
25 changes: 25 additions & 0 deletions packages/api/tests/animal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,31 @@ describe('Animal Tests', () => {
message: 'Default breed must use default type',
},
},
{
testName: 'Cannot create a duplicate custom type',
getPostBody: (customs) => [
{
type_name: customs.customAnimalType.type,
},
],
postErr: {
code: 409,
message: 'Animal type already exists',
},
},
{
testName: 'Cannot create a duplicate custom breed',
getPostBody: (customs) => [
{
default_type_id: customs.customAnimalBreed.default_type_id,
breed_name: customs.customAnimalBreed.breed,
},
],
postErr: {
code: 409,
message: 'Animal breed already exists',
},
},
],
EDIT: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ export default function AddAnimalsFormCard({
control={control}
breedOptions={filteredBreeds}
isTypeSelected={!!watchAnimalType}
onBreedChange={(option) => {
trigger(`${namePrefix}${BasicsFields.BREED}`);
}}
error={get(errors, `${namePrefix}${BasicsFields.BREED}`)}
/>

<div className={styles.countAndSexDetailsWrapper}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ export function AnimalTypeSelect<T extends FieldValues>({
<Controller
name={name}
control={control}
rules={{ required: { value: true, message: t('common:REQUIRED') } }}
rules={{
required: { value: true, message: t('common:REQUIRED') },
}}
render={({ field: { onChange, value } }) => (
<CreatableSelect
label={t('ADD_ANIMAL.TYPE')}
Expand All @@ -58,6 +60,7 @@ export function AnimalTypeSelect<T extends FieldValues>({
}}
value={value}
isDisabled={isDisabled}
isClearable={false}
/>
)}
/>
Expand All @@ -71,6 +74,8 @@ export type AnimalBreedSelectProps = {
isTypeSelected?: boolean;
breedSelectRef?: RefObject<SelectInstance>;
isDisabled?: boolean;
error?: FieldError;
onBreedChange?: (Option: Option | null) => void;
};

export function AnimalBreedSelect<T extends FieldValues>({
Expand All @@ -80,25 +85,35 @@ export function AnimalBreedSelect<T extends FieldValues>({
isTypeSelected,
breedSelectRef,
isDisabled = false,
error,
onBreedChange,
}: AnimalBreedSelectProps & UseControllerProps<T>) {
const { t } = useTranslation();
return (
<Controller
name={name}
control={control}
render={({ field: { onChange, value } }) => (
<CreatableSelect
ref={breedSelectRef}
options={breedOptions}
label={t('ADD_ANIMAL.BREED')}
optional
controlShouldRenderValue={isTypeSelected}
placeholder={isTypeSelected ? undefined : t('ADD_ANIMAL.BREED_PLACEHOLDER_DISABLED')}
isDisabled={!isTypeSelected || isDisabled}
onChange={(option) => onChange(option)}
value={value || null}
/>
)}
/>
<div>
<Controller
name={name}
control={control}
render={({ field: { onChange, value } }) => (
<CreatableSelect
ref={breedSelectRef}
options={breedOptions}
label={t('ADD_ANIMAL.BREED')}
optional
controlShouldRenderValue={isTypeSelected}
placeholder={isTypeSelected ? undefined : t('ADD_ANIMAL.BREED_PLACEHOLDER_DISABLED')}
isDisabled={!isTypeSelected || isDisabled}
onChange={(option) => {
onChange(option);
// @ts-ignore
onBreedChange?.(option);
}}
value={value || null}
isClearable={false}
/>
)}
/>
{error && <Error>{error.message}</Error>}
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ import { useTranslation } from 'react-i18next';
import { ClearIndicator, MultiValueRemove, MenuOpenDropdownIndicator } from './components';
import scss from './styles.module.scss';

const upperCaseTrim = (a: String) => {
return a.toUpperCase().trim();
};

/**
* Check for duplicate or matching strings - (same as backend util)
* TODO: consider localeCompare() or not caring about case sensitivity
**/

export const compareUpperCaseTrim = (a: String, b: String) => {
return upperCaseTrim(a) === upperCaseTrim(b);
};

type CreatableSelectProps<
Option = unknown,
IsMulti extends boolean = false,
Expand Down Expand Up @@ -55,11 +68,17 @@ const CreatableSelect = React.forwardRef((props, ref) => {
components,
createPromptText = t('common:CREATE'),
placeholder = t('common:SELECT_OR_ADD_YOUR_OWN'),
options,
...restProps
} = props;

const isValidNewOption = (inputValue: string) => {
return inputValue.trim().length > 0;
return (
inputValue.trim().length > 0 &&
!options?.some((opt) => {
// @ts-ignore
return compareUpperCaseTrim(opt?.label, inputValue);
})
);
};

return (
Expand Down Expand Up @@ -91,6 +110,7 @@ const CreatableSelect = React.forwardRef((props, ref) => {
...components,
}}
ref={ref}
options={options}
{...restProps}
/>
</div>
Expand Down

0 comments on commit e27e81d

Please sign in to comment.