Skip to content
New issue

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

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

Already on GitHub? # to your account

assignToCurrentChannel publishes ChangeChannelEvent prematurely #3166

Open
DanielBiegler opened this issue Oct 27, 2024 · 0 comments
Open

assignToCurrentChannel publishes ChangeChannelEvent prematurely #3166

DanielBiegler opened this issue Oct 27, 2024 · 0 comments
Labels
type: bug 🐛 Something isn't working

Comments

@DanielBiegler
Copy link
Contributor

Describe the bug

The following function inside the ChannelService:

    /**
     * @description
     * Assigns a ChannelAware entity to the default Channel as well as any channel
     * specified in the RequestContext.
     */
    async assignToCurrentChannel<T extends ChannelAware & VendureEntity>(
        entity: T,
        ctx: RequestContext,
    ): Promise<T> {
        const defaultChannel = await this.getDefaultChannel(ctx);
        const channelIds = unique([ctx.channelId, defaultChannel.id]);
        entity.channels = channelIds.map(id => ({ id })) as any;
        await this.eventBus.publish(new ChangeChannelEvent(ctx, entity, [ctx.channelId], 'assigned'));
        return entity;
    }

The function doesnt actually persist anything to the DB, it only patches the passed in object which you have to save separately. So if something goes wrong before or when saving your entity, you still publish the event which can lead to bugs if you have logic thats dependent on the ChangeChannelEvent as now your entity has in fact, not been assigned to a new channel. In the event itself the entity will have .channels assigned though, so everything will look proper but be wrong.

To Reproduce

  1. Subscribe to the ChangeChannelEvent
  2. Call assignToCurrentChannel on some entity but then fail to persist it
  3. Now your subscriber got called with a wrong event

Expected behavior

The event should only be published after actually persisting to the db. This will require a refactor of many services but keep in mind that plugins that involuntarily relied on this will be affected too. ☹️

@DanielBiegler DanielBiegler added the type: bug 🐛 Something isn't working label Oct 27, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant