-
Notifications
You must be signed in to change notification settings - Fork 1
#64 get, create and edit projects #80
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
Conversation
jorgecod
commented
Apr 7, 2020
- Get, Create and Edit Users
src/app/app.module.ts
Outdated
| StoreModule.forRoot(reducers, { | ||
| metaReducers, | ||
| }), | ||
| !environment.production ? StoreDevtoolsModule.instrument() : [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this line replace lines 78-79-80?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this line
| component.isEdit = false; | ||
| spyOn(store, 'dispatch'); | ||
| component.onSubmit(project); | ||
| expect(store.dispatch).toHaveBeenCalledWith(new actions.PostProject(project)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a better action name would be CreateProject(project)
| spyOn(store, 'dispatch'); | ||
| component.onSubmit(project); | ||
| expect(component.savedProject.emit).toHaveBeenCalled(); | ||
| expect(store.dispatch).toHaveBeenCalledWith(new actions.PutProject(project)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change from PutProject to UpdateProject
|
|
||
| it('details field validity', () => { | ||
| const details = component.projectForm.controls.details; | ||
| it('description field validity', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's change the test name to:
it('checks if description field is valid',
I know it's not your code but let's fix this.
| @Output() cancelForm = new EventEmitter(); | ||
|
|
||
| constructor(private formBuilder: FormBuilder) { | ||
| constructor(private formBuilder: FormBuilder, private store: Store<any>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store<any> ?
I think we have to use Store<ProjectsStore> or so
| constructor(public payload: Project) {} | ||
| } | ||
|
|
||
| export class PutProjectSuccess implements Action { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UpdateProjectSuccess
| constructor(public payload: Project) {} | ||
| } | ||
|
|
||
| export class PutProjectFail implements Action { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UpdateProjectFail
| ); | ||
|
|
||
| @Effect() | ||
| postProject$: Observable<Action> = this.actions$.pipe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createProject$
| ); | ||
|
|
||
| @Effect() | ||
| putProject$: Observable<Action> = this.actions$.pipe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateProject$
| isLoading: boolean; | ||
|
|
||
| constructor(private projectService: ProjectService) { | ||
| constructor(private store: Store<any>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store => Store
|
|
||
| @Injectable() | ||
| export class ProjectEffects { | ||
| constructor(private actions$: Actions, private projectService: ProjectService, private store: Store<any>) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're not using the store at all. Why are we injecting it here?