Skip to content

Conversation

ivanportillo
Copy link
Contributor

No description provided.

aliondev and others added 11 commits June 9, 2020 00:01
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: Iván Portillo <ivanportillo@me.com>
Co-authored-by: aliondev <antonioleon@audiense.com>
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: aliondev <antonioleon@audiense.com>
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: aliondev <antonioleon@audiense.com>
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: aliondev <antonioleon@audiense.com>
Co-authored-by: Rubén Salado <rsaladocid@users.noreply.github.com>
Co-authored-by: aliondev <antonioleon@audiense.com>
Copy link
Member

@JavierCane JavierCane left a comment

Choose a reason for hiding this comment

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

🙌🙌🙌

Comment on lines +3 to +6
export interface CommandHandler<T extends Command> {
subscribedTo(): Command;
handle(command: T): 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.

😍😊


export interface CommandBus {
dispatch(command: Command): Promise<void>;
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about enabling the a TSLint eofline rule?

Right now it's disabled.

Also, we should add the tslint execution to the npm test or some other task to be executed in the project GitHub Workflow.

Do you agree? Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree, I'm going to add it

Comment on lines 4 to 6
export interface QueryBus {
ask<R extends Response>(query: Query): Promise<R>;
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

👌👌👌

Comment on lines +26 to +28
if (!queryHandler) {
throw new QueryNotRegisteredError(query);
}
Copy link
Member

Choose a reason for hiding this comment

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

Are we handling these exception somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This exception and the CommandNotRegisteredError are handled by the controller, throwing a 500 error. Is not handled as a specific error but its a general error.

https://github.com/CodelyTV/typescript-ddd-skeleton/blob/d6b7729e96f7ef64681101151e794f38913ed9d9/src/apps/mooc_backend/controllers/CoursesCounterGetController.ts#L21

@ivanportillo ivanportillo changed the base branch from master to part_four July 10, 2020 15:28
@ivanportillo ivanportillo force-pushed the part_four_query_bus branch 2 times, most recently from 8a0359a to 2e2c8c1 Compare July 10, 2020 15:50
@ivanportillo ivanportillo force-pushed the part_four_query_bus branch from 2e2c8c1 to 80cb15f Compare July 10, 2020 15:52
@ivanportillo ivanportillo merged commit 85e6769 into part_four Jul 10, 2020
@ivanportillo ivanportillo deleted the part_four_query_bus branch July 10, 2020 15:59
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.

4 participants