Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: NLP module strict null checks issues #544

Merged
2 changes: 1 addition & 1 deletion api/src/helper/lib/base-nlp-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default abstract class BaseNlpHelper<
})
.concat({
entity: 'language',
value: s.language.code,
value: s.language!.code,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't language supposed to be mandatory

Copy link
Contributor

Choose a reason for hiding this comment

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

@yassinedorbozgithub @MohamedAliBouhaouala Can we open a Issue to force the user to set the language for each sample ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Issue created ✅ #561


return {
Expand Down
14 changes: 13 additions & 1 deletion api/src/i18n/services/language.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import {
Inject,
Injectable,
InternalServerErrorException,
NotFoundException,
} from '@nestjs/common';
import { Cache } from 'cache-manager';

import { LoggerService } from '@/logger/logger.service';
import {
DEFAULT_LANGUAGE_CACHE_KEY,
LANGUAGES_CACHE_KEY,
Expand All @@ -35,6 +37,7 @@ export class LanguageService extends BaseService<
constructor(
readonly repository: LanguageRepository,
@Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
private readonly logger: LoggerService,
) {
super(repository);
}
Expand Down Expand Up @@ -78,6 +81,15 @@ export class LanguageService extends BaseService<
* @returns A promise that resolves to the `Language` object.
*/
async getLanguageByCode(code: string) {
return await this.findOne({ code });
const language = await this.findOne({ code });

if (!language) {
this.logger.warn(`Unable to Language by languageCode ${code}`);
throw new NotFoundException(
`Language with languageCode ${code} not found`,
);
}

return language;
}
}
65 changes: 38 additions & 27 deletions api/src/nlp/controllers/nlp-entity.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@ import {
closeInMongodConnection,
rootMongooseTestModule,
} from '@/utils/test/test';
import { TFixtures } from '@/utils/test/types';

import { NlpEntityCreateDto } from '../dto/nlp-entity.dto';
import { NlpEntityRepository } from '../repositories/nlp-entity.repository';
import { NlpSampleEntityRepository } from '../repositories/nlp-sample-entity.repository';
import { NlpValueRepository } from '../repositories/nlp-value.repository';
import {
NlpEntityModel,
NlpEntity,
NlpEntityFull,
NlpEntityModel,
} from '../schemas/nlp-entity.schema';
import { NlpSampleEntityModel } from '../schemas/nlp-sample-entity.schema';
import { NlpValueModel } from '../schemas/nlp-value.schema';
Expand All @@ -48,8 +49,8 @@ describe('NlpEntityController', () => {
let nlpEntityController: NlpEntityController;
let nlpValueService: NlpValueService;
let nlpEntityService: NlpEntityService;
let intentEntityId: string;
let buitInEntityId: string;
let intentEntityId: string | null;
let buitInEntityId: string | null;

beforeAll(async () => {
const module: TestingModule = await Test.createTestingModule({
Expand All @@ -76,16 +77,18 @@ describe('NlpEntityController', () => {
nlpValueService = module.get<NlpValueService>(NlpValueService);
nlpEntityService = module.get<NlpEntityService>(NlpEntityService);

intentEntityId = (
await nlpEntityService.findOne({
name: 'intent',
})
).id;
buitInEntityId = (
await nlpEntityService.findOne({
name: 'built_in',
})
).id;
intentEntityId =
(
await nlpEntityService.findOne({
name: 'intent',
})
)?.id || null;
buitInEntityId =
(
await nlpEntityService.findOne({
name: 'built_in',
})
)?.id || null;
});
afterAll(async () => {
await closeInMongodConnection();
Expand All @@ -107,11 +110,13 @@ describe('NlpEntityController', () => {
...curr,
values: nlpValueFixtures.filter(
({ entity }) => parseInt(entity) === index,
),
) as NlpEntityFull['values'],
lookups: curr.lookups!,
builtin: curr.builtin!,
});
return acc;
},
[],
[] as TFixtures<NlpEntityFull>[],
);
expect(result).toEqualPayload(
entitiesWithValues.sort((a, b) => {
Expand Down Expand Up @@ -170,19 +175,19 @@ describe('NlpEntityController', () => {

describe('deleteOne', () => {
it('should delete a nlp entity', async () => {
const result = await nlpEntityController.deleteOne(intentEntityId);
const result = await nlpEntityController.deleteOne(intentEntityId!);
expect(result.deletedCount).toEqual(1);
});

it('should throw exception when nlp entity id not found', async () => {
await expect(
nlpEntityController.deleteOne(intentEntityId),
nlpEntityController.deleteOne(intentEntityId!),
).rejects.toThrow(NotFoundException);
});

it('should throw exception when nlp entity is builtin', async () => {
await expect(
nlpEntityController.deleteOne(buitInEntityId),
nlpEntityController.deleteOne(buitInEntityId!),
).rejects.toThrow(MethodNotAllowedException);
});
});
Expand All @@ -192,10 +197,10 @@ describe('NlpEntityController', () => {
const firstNameEntity = await nlpEntityService.findOne({
name: 'first_name',
});
const result = await nlpEntityController.findOne(firstNameEntity.id, []);
const result = await nlpEntityController.findOne(firstNameEntity!.id, []);

expect(result).toEqualPayload(
nlpEntityFixtures.find(({ name }) => name === 'first_name'),
nlpEntityFixtures.find(({ name }) => name === 'first_name')!,
);
});

Expand All @@ -206,17 +211,23 @@ describe('NlpEntityController', () => {
const firstNameValues = await nlpValueService.findOne({ value: 'jhon' });
const firstNameWithValues: NlpEntityFull = {
...firstNameEntity,
values: [firstNameValues],
values: firstNameValues ? [firstNameValues] : [],
name: firstNameEntity!.name,
id: firstNameEntity!.id,
createdAt: firstNameEntity!.createdAt,
updatedAt: firstNameEntity!.updatedAt,
lookups: firstNameEntity!.lookups,
builtin: firstNameEntity!.builtin,
};
const result = await nlpEntityController.findOne(firstNameEntity.id, [
const result = await nlpEntityController.findOne(firstNameEntity!.id, [
'values',
]);
expect(result).toEqualPayload(firstNameWithValues);
});

it('should throw NotFoundException when Id does not exist', async () => {
await expect(
nlpEntityController.findOne(intentEntityId, ['values']),
nlpEntityController.findOne(intentEntityId!, ['values']),
).rejects.toThrow(NotFoundException);
});
});
Expand All @@ -233,7 +244,7 @@ describe('NlpEntityController', () => {
builtin: false,
};
const result = await nlpEntityController.updateOne(
firstNameEntity.id,
firstNameEntity!.id,
updatedNlpEntity,
);
expect(result).toEqualPayload(updatedNlpEntity);
Expand All @@ -247,7 +258,7 @@ describe('NlpEntityController', () => {
builtin: false,
};
await expect(
nlpEntityController.updateOne(intentEntityId, updateNlpEntity),
nlpEntityController.updateOne(intentEntityId!, updateNlpEntity),
).rejects.toThrow(NotFoundException);
});

Expand All @@ -259,7 +270,7 @@ describe('NlpEntityController', () => {
builtin: false,
};
await expect(
nlpEntityController.updateOne(buitInEntityId, updateNlpEntity),
nlpEntityController.updateOne(buitInEntityId!, updateNlpEntity),
).rejects.toThrow(MethodNotAllowedException);
});
});
Expand All @@ -276,7 +287,7 @@ describe('NlpEntityController', () => {
name: 'updated',
})
)?.id,
];
] as string[];

const result = await nlpEntityController.deleteMany(entitiesToDelete);

Expand Down
Loading
Loading