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: apply strict null checks updates to the Chat Module #547

Merged
merged 15 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/channel/channel.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ export class ChannelService {
},
{
foreign_id: req.session.passport.user.id,
first_name: req.session.passport.user.first_name,
last_name: req.session.passport.user.last_name,
first_name: req.session.passport.user.first_name || 'Anonymous',
last_name: req.session.passport.user.last_name || 'Anonymous',
locale: '',
language: '',
gender: '',
Expand Down
2 changes: 2 additions & 0 deletions api/src/channel/lib/__test__/subscriber.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export const subscriberInstance: Subscriber = {
name: 'web-channel',
},
labels: [],
avatar: null,
context: {},
...modelInstance,
};

Expand Down
20 changes: 12 additions & 8 deletions api/src/chat/controllers/block.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,19 @@ describe('BlockController', () => {
blockController = module.get<BlockController>(BlockController);
blockService = module.get<BlockService>(BlockService);
categoryService = module.get<CategoryService>(CategoryService);
category = await categoryService.findOne({ label: 'default' });
block = await blockService.findOne({ name: 'first' });
blockToDelete = await blockService.findOne({ name: 'buttons' });
hasNextBlocks = await blockService.findOne({
category = (await categoryService.findOne({ label: 'default' }))!;
block = (await blockService.findOne({ name: 'first' }))!;
blockToDelete = (await blockService.findOne({ name: 'buttons' }))!;
hasNextBlocks = (await blockService.findOne({
name: 'hasNextBlocks',
});
hasPreviousBlocks = await blockService.findOne({
}))!;
hasPreviousBlocks = (await blockService.findOne({
name: 'hasPreviousBlocks',
});
}))!;
});

afterEach(jest.clearAllMocks);

afterAll(closeInMongodConnection);

describe('find', () => {
Expand Down Expand Up @@ -185,6 +186,7 @@ describe('BlockController', () => {
blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [],
nextBlocks:
blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [],
attachedToBlock: null,
}));

expect(blockService.findAndPopulate).toHaveBeenCalledWith({}, undefined);
Expand Down Expand Up @@ -220,12 +222,13 @@ describe('BlockController', () => {
...blockFixtures.find(({ name }) => name === 'hasPreviousBlocks'),
category,
previousBlocks: [hasNextBlocks],
attachedToBlock: null,
});
});

it('should find one block by id, and populate its category and an empty previousBlocks', async () => {
jest.spyOn(blockService, 'findOneAndPopulate');
block = await blockService.findOne({ name: 'attachment' });
block = (await blockService.findOne({ name: 'attachment' }))!;
const result = await blockController.findOne(
block.id,
FIELDS_TO_POPULATE,
Expand All @@ -235,6 +238,7 @@ describe('BlockController', () => {
...blockFixtures.find(({ name }) => name === 'attachment'),
category,
previousBlocks: [],
attachedToBlock: null,
});
});
});
Expand Down
9 changes: 6 additions & 3 deletions api/src/chat/controllers/block.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,12 @@ export class BlockController extends BaseController<
this.validate({
dto: block,
allowedIds: {
category: (await this.categoryService.findOne(block.category))?.id,
attachedBlock: (await this.blockService.findOne(block.attachedBlock))
?.id,
category: block.category
? (await this.categoryService.findOne(block.category))?.id
: null,
attachedBlock: block.attachedBlock
? (await this.blockService.findOne(block.attachedBlock))?.id
: null,
nextBlocks: (
await this.blockService.find({
_id: {
Expand Down
11 changes: 6 additions & 5 deletions api/src/chat/controllers/context-var.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,16 @@ describe('ContextVarController', () => {
contextVarController =
module.get<ContextVarController>(ContextVarController);
contextVarService = module.get<ContextVarService>(ContextVarService);
contextVar = await contextVarService.findOne({
contextVar = (await contextVarService.findOne({
label: 'test context var 1',
});
contextVarToDelete = await contextVarService.findOne({
}))!;
contextVarToDelete = (await contextVarService.findOne({
label: 'test context var 2',
});
}))!;
});

afterEach(jest.clearAllMocks);

afterAll(closeInMongodConnection);

describe('count', () => {
Expand Down Expand Up @@ -95,7 +96,7 @@ describe('ContextVarController', () => {

expect(contextVarService.findOne).toHaveBeenCalledWith(contextVar.id);
expect(result).toEqualPayload(
contextVarFixtures.find(({ label }) => label === contextVar.label),
contextVarFixtures.find(({ label }) => label === contextVar.label)!,
);
});
});
Expand Down
23 changes: 12 additions & 11 deletions api/src/chat/controllers/message.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,17 @@ describe('MessageController', () => {
userService = module.get<UserService>(UserService);
subscriberService = module.get<SubscriberService>(SubscriberService);
messageController = module.get<MessageController>(MessageController);
message = await messageService.findOne({ mid: 'mid-1' });
sender = await subscriberService.findOne(message.sender);
recipient = await subscriberService.findOne(message.recipient);
user = await userService.findOne(message.sentBy);
message = (await messageService.findOne({ mid: 'mid-1' }))!;
sender = (await subscriberService.findOne(message.sender!))!;
recipient = (await subscriberService.findOne(message.recipient!))!;
user = (await userService.findOne(message.sentBy!))!;
allSubscribers = await subscriberService.findAll();
allUsers = await userService.findAll();
allMessages = await messageService.findAll();
});

afterEach(jest.clearAllMocks);

afterAll(closeInMongodConnection);

describe('count', () => {
Expand Down Expand Up @@ -189,10 +190,10 @@ describe('MessageController', () => {
const result = await messageController.findPage(pageQuery, [], {});
const messagesWithSenderAndRecipient = allMessages.map((message) => ({
...message,
sender: allSubscribers.find(({ id }) => id === message['sender']).id,
recipient: allSubscribers.find(({ id }) => id === message['recipient'])
.id,
sentBy: allUsers.find(({ id }) => id === message['sentBy']).id,
sender: allSubscribers.find(({ id }) => id === message.sender)?.id,
recipient: allSubscribers.find(({ id }) => id === message.recipient)
?.id,
sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id,
}));

expect(messageService.find).toHaveBeenCalledWith({}, pageQuery);
Expand All @@ -208,9 +209,9 @@ describe('MessageController', () => {
);
const messages = allMessages.map((message) => ({
...message,
sender: allSubscribers.find(({ id }) => id === message['sender']),
recipient: allSubscribers.find(({ id }) => id === message['recipient']),
sentBy: allUsers.find(({ id }) => id === message['sentBy']).id,
sender: allSubscribers.find(({ id }) => id === message.sender),
recipient: allSubscribers.find(({ id }) => id === message.recipient),
sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id,
}));

expect(messageService.findAndPopulate).toHaveBeenCalledWith(
Expand Down
7 changes: 4 additions & 3 deletions api/src/chat/controllers/subscriber.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,16 @@ describe('SubscriberController', () => {

subscriberController =
module.get<SubscriberController>(SubscriberController);
subscriber = await subscriberService.findOne({
subscriber = (await subscriberService.findOne({
first_name: 'Jhon',
});
}))!;
allLabels = await labelService.findAll();
allSubscribers = await subscriberService.findAll();
allUsers = await userService.findAll();
});

afterEach(jest.clearAllMocks);

afterAll(closeInMongodConnection);

describe('count', () => {
Expand All @@ -125,7 +126,7 @@ describe('SubscriberController', () => {
({ first_name }) => first_name === subscriber.first_name,
),
labels: labelIDs,
assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id).id,
assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id)?.id,
});
});

Expand Down
4 changes: 3 additions & 1 deletion api/src/chat/controllers/subscriber.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ export class SubscriberController extends BaseController<
* @returns A streamable file containing the avatar image.
*/
@Get(':id/profile_pic')
async getAvatar(@Param('id') id: string): Promise<StreamableFile> {
async getAvatar(
@Param('id') id: string,
): Promise<StreamableFile | undefined> {
const subscriber = await this.subscriberService.findOneAndPopulate(id);

if (!subscriber) {
Expand Down
4 changes: 2 additions & 2 deletions api/src/chat/dto/block.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ export class BlockCreateDto {
@IsObjectId({
message: 'Attached block must be a valid objectId',
})
attachedBlock?: string;
attachedBlock?: string | null;

@ApiProperty({ description: 'Block category', type: String })
@IsNotEmpty()
@IsString()
@IsObjectId({ message: 'Category must be a valid objectId' })
category: string;
category: string | null;

@ApiPropertyOptional({
description: 'Block has started conversation',
Expand Down
17 changes: 10 additions & 7 deletions api/src/chat/repositories/block.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ describe('BlockRepository', () => {
validIds = ['64abc1234def567890fedcba', '64abc1234def567890fedcbc'];
validCategory = '64def5678abc123490fedcba';

category = await categoryRepository.findOne({ label: 'default' });
hasPreviousBlocks = await blockRepository.findOne({
category = (await categoryRepository.findOne({ label: 'default' }))!;
hasPreviousBlocks = (await blockRepository.findOne({
name: 'hasPreviousBlocks',
});
hasNextBlocks = await blockRepository.findOne({
}))!;
hasNextBlocks = (await blockRepository.findOne({
name: 'hasNextBlocks',
});
}))!;
});

afterEach(jest.clearAllMocks);
Expand All @@ -77,6 +77,7 @@ describe('BlockRepository', () => {
category,
nextBlocks: [hasPreviousBlocks],
previousBlocks: [],
attachedToBlock: null,
});
});
});
Expand All @@ -93,6 +94,7 @@ describe('BlockRepository', () => {
blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [],
nextBlocks:
blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [],
attachedToBlock: null,
}));

expect(blockModel.find).toHaveBeenCalledWith({}, undefined);
Expand All @@ -110,6 +112,7 @@ describe('BlockRepository', () => {
blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [],
nextBlocks:
blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [],
attachedToBlock: null,
}));

expect(blockModel.find).toHaveBeenCalledWith({}, undefined);
Expand Down Expand Up @@ -191,7 +194,7 @@ describe('BlockRepository', () => {
category: validCategory,
nextBlocks: [],
attachedBlock: null,
} as Block);
} as unknown as Block);

const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');

Expand Down Expand Up @@ -233,7 +236,7 @@ describe('BlockRepository', () => {
attachedBlock: null,
nextBlocks: [validIds[0], validIds[1]],
},
] as Block[];
] as unknown as Block[];

const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');

Expand Down
2 changes: 1 addition & 1 deletion api/src/chat/repositories/context-var.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class ContextVarRepository extends BaseRepository<
@Optional() blockService?: BlockService,
) {
super(eventEmitter, model, ContextVar);
this.blockService = blockService;
if (blockService) this.blockService = blockService;
}

/**
Expand Down
10 changes: 5 additions & 5 deletions api/src/chat/repositories/message.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ describe('MessageRepository', () => {
describe('findOneAndPopulate', () => {
it('should find one message by id, and populate its sender and recipient', async () => {
jest.spyOn(messageModel, 'findById');
const message = await messageRepository.findOne({ mid: 'mid-1' });
const sender = await subscriberRepository.findOne(message['sender']);
const message = (await messageRepository.findOne({ mid: 'mid-1' }))!;
const sender = await subscriberRepository.findOne(message!['sender']);
const recipient = await subscriberRepository.findOne(
message['recipient'],
message!['recipient'],
);
const user = await userRepository.findOne(message['sentBy']);
const user = (await userRepository.findOne(message!['sentBy']))!;
const result = await messageRepository.findOneAndPopulate(message.id);

expect(messageModel.findById).toHaveBeenCalledWith(message.id, undefined);
Expand All @@ -92,7 +92,7 @@ describe('MessageRepository', () => {
...message,
sender: allSubscribers.find(({ id }) => id === message['sender']),
recipient: allSubscribers.find(({ id }) => id === message['recipient']),
sentBy: allUsers.find(({ id }) => id === message['sentBy']).id,
sentBy: allUsers.find(({ id }) => id === message['sentBy'])?.id,
}));

expect(messageModel.find).toHaveBeenCalledWith({}, undefined);
Expand Down
5 changes: 3 additions & 2 deletions api/src/chat/repositories/subscriber.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,15 @@ describe('SubscriberRepository', () => {
});

afterEach(jest.clearAllMocks);

afterAll(closeInMongodConnection);

describe('findOneAndPopulate', () => {
it('should find one subscriber by id,and populate its labels', async () => {
jest.spyOn(subscriberModel, 'findById');
const subscriber = await subscriberRepository.findOne({
const subscriber = (await subscriberRepository.findOne({
first_name: 'Jhon',
});
}))!;
const allLabels = await labelRepository.findAll();
const result = await subscriberRepository.findOneAndPopulate(
subscriber.id,
Expand Down
10 changes: 6 additions & 4 deletions api/src/chat/repositories/subscriber.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export class SubscriberRepository extends BaseRepository<
*/
async findOneByForeignIdAndPopulate(id: string): Promise<SubscriberFull> {
const query = this.findByForeignIdQuery(id).populate(this.populate);
const [result] = await this.execute(query, this.clsPopulate);
const [result] = await this.execute(query, SubscriberFull);
return result;
}

Expand All @@ -150,7 +150,7 @@ export class SubscriberRepository extends BaseRepository<
async updateOneByForeignIdQuery(
id: string,
updates: SubscriberUpdateDto,
): Promise<Subscriber> {
): Promise<Subscriber | null> {
return await this.updateOne({ foreign_id: id }, updates);
}

Expand All @@ -161,7 +161,9 @@ export class SubscriberRepository extends BaseRepository<
*
* @returns The updated subscriber entity.
*/
async handBackByForeignIdQuery(foreignId: string): Promise<Subscriber> {
async handBackByForeignIdQuery(
foreignId: string,
): Promise<Subscriber | null> {
return await this.updateOne(
{
foreign_id: foreignId,
Expand All @@ -184,7 +186,7 @@ export class SubscriberRepository extends BaseRepository<
async handOverByForeignIdQuery(
foreignId: string,
userId: string,
): Promise<Subscriber> {
): Promise<Subscriber | null> {
return await this.updateOne(
{
foreign_id: foreignId,
Expand Down
Loading
Loading