Skip to content

Conversation

johnmasking
Copy link
Member

Fixes #342

Changes proposed in this pull request:

  • Added subscription to the post deleted event channel/name
  • Added getByPostId function
  • Added deleteData function
  • Added test

@MaskingTechnology/comify

@johnmasking johnmasking linked an issue Mar 21, 2025 that may be closed by this pull request
@petermasking petermasking changed the title #342 (feat domain final code + tests feat(domain): auto delete notifications Mar 21, 2025
Copy link
Member

@petermasking petermasking left a comment

Choose a reason for hiding this comment

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

Almost there! Just a few things. Also, the notification getById feature doesn't include the deleted flag yet.

{
const notifications = await getNotifications(postId);

return notifications.map(item => deleteData(item.id));
Copy link
Member

@petermasking petermasking Mar 21, 2025

Choose a reason for hiding this comment

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

Suggested change
return notifications.map(item => deleteData(item.id));
const promises = notifications.map(item => deleteData(item.id));
await Promise.allSettled(promises);

Return type: Promise<void>

Copy link
Member

Choose a reason for hiding this comment

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

This file contains the actual feature, and therefore should be named remove.ts.

@@ -0,0 +1,2 @@

export { default } from './deleteData';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export { default } from './deleteData';
export { default } from './remove';


export default async function remove(id: string): Promise<void>
{
return database.updateRecord(RECORD_TYPE, id, { deleted: true }) as Promise<void>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return database.updateRecord(RECORD_TYPE, id, { deleted: true }) as Promise<void>;
return database.updateRecord(RECORD_TYPE, id, { deleted: true });


import database from '^/integrations/database';

import type { RecordQuery } from '^/integrations/database';
Copy link
Member

Choose a reason for hiding this comment

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

The import should be grouped with the other ^/integration/... import.


import database from '^/integrations/database';

import type { RecordQuery } from '^/integrations/database';
Copy link
Member

Choose a reason for hiding this comment

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

The import should be grouped with the other ^/integration/... import.

Copy link
Member

Choose a reason for hiding this comment

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

The file tests the remove feature of the notification. The name of the file should reflect this.


expect(result).toHaveLength(0);
});

Copy link
Member

Choose a reason for hiding this comment

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

This empty line is not required


import getByPostId from '^/domain/notification/getByPostId';
import removedPost from '^/domain/notification/notify/removedPost';
import { DATABASES, VALUES } from './fixtures';
Copy link
Member

Choose a reason for hiding this comment

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

There should be an empty line before this import. Those above are coming from the domain, and this one from ./

Copy link

@basmasking basmasking merged commit a228ad2 into main Mar 26, 2025
6 checks passed
@basmasking basmasking deleted the 342-delete-notification-when-deleting-related-item branch March 26, 2025 11:41
@johnmasking johnmasking restored the 342-delete-notification-when-deleting-related-item branch March 27, 2025 22:14
@basmasking basmasking deleted the 342-delete-notification-when-deleting-related-item branch June 13, 2025 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete notification when deleting related item

3 participants