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

feat: implement control-plane service and testing #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Soni295
Copy link

@Soni295 Soni295 commented Apr 5, 2023

Hola Alan. Probé hacer el desafió, solo que no estaría seguro si tuviera que agregar un logger en el control en que nivel del código tendría que ponerlo y no estaría muy seguro que nombres poner a las interfaces y clases aun. Muy buenos vídeos che, un saludo.

Copy link
Contributor

@Alan-TheGentleman Alan-TheGentleman left a comment

Choose a reason for hiding this comment

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

Me ha gustado mucho !
Algunas observaciones para terminarlo correctamente aparte de los comentarios fijados son:

  • Ojo con los lugares donde haces la lógica de negocio, los adapters solo deben de poseer mapeos de datos para que la información que entre sea util para la app y la que salga para el hexágono externo

PD: No mergear, es solo a nivel de aprendizaje

];

export class ControlAuthenticatorStub implements ForAuthenticating {
async getAuthDetails(email: string, password: string): Promise<AuthDetails> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Esto está muy bien, si quieres puedes poner que siempre devuelva un mock sin necesidad de agregar lógica de búsqueda ya que justamente es eso, es para controlar la lógica principal en un ambiente controlado

Copy link
Contributor

Choose a reason for hiding this comment

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

La lógica en sí deberías de hacerla en el control plane ya que es parte del domain


async getAuthDetails(email: string, password: string): Promise<AuthDetails> {
return this.controlPlane.getAuthDetails(email, password);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Agregar espacio entre métodos y secciones lógicas

Suggested change
}
}


export const compositionMock = () => {
const authenticatorStub = new ControlAuthenticatorStub();
const controlPlane = new ControlPlane(authenticatorStub);
Copy link
Contributor

Choose a reason for hiding this comment

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

llamarlo controlPlaneMock, ya que justamente es un mock del control plane

const controlAuthenticatingProxyAdapter =
new ControlAuthenticatingProxyAdapter(controlPlane);

it.concurrent("should return a token an refreshToken", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

faltan los
//GIVEN
//WHEN
//THEN

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.

2 participants