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

[#1615] Expose tag events via websocket #1624

Merged
merged 14 commits into from Apr 29, 2021
Merged

Conversation

lucapette
Copy link
Contributor

No description provided.

@lucapette lucapette changed the title Fist pass at tag events [#1615] Expose tag events via websocket Apr 23, 2021
@lucapette lucapette force-pushed the feat/1615-tags-event branch 3 times, most recently from d540a2e to 82a4e08 Compare April 26, 2021 14:18
AitorAlgorta
AitorAlgorta previously approved these changes Apr 29, 2021
Copy link
Contributor

@AitorAlgorta AitorAlgorta left a comment

Choose a reason for hiding this comment

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

Lovely

@lucapette lucapette marked this pull request as ready for review April 29, 2021 08:59
AitorAlgorta
AitorAlgorta previously approved these changes Apr 29, 2021
chrismatix
chrismatix previously approved these changes Apr 29, 2021
Copy link
Contributor

@chrismatix chrismatix left a comment

Choose a reason for hiding this comment

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

Very cool refactoring. Left some comments

delivery_state: DeliveryState;
from_contact: boolean;
sent_at: Date;
metadata: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Message events don't include metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this was already there bu I agree. I'll change!


```json5
{
"type": "tag",
Copy link
Contributor

Choose a reason for hiding this comment

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

So for my understanding: For now we don't handle tag deletions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the reason why I didn't do this is that right now "deleting tags" is not though off so well as it doesn't cascade to conversations (and maybe we should never do that. I guess we should discuss this in a separate issue though)

const SET_METADATA = '@@metadata/SET_METADATA';

export const setMetadataAction = createAction(
SET_METADATA,
(metadataEvent: MetadataEvent) => metadataEvent
)<MetadataEvent>();

export const mergeMetadataAction = createAction(
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 So this is also a step on the way to default values on the client.

@lucapette lucapette merged commit 97666de into develop Apr 29, 2021
@lucapette lucapette deleted the feat/1615-tags-event branch April 29, 2021 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants