Update App Summary entity after carrying out App Route actions #2548
Conversation
|
Hey irfanhabib! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## v2-master #2548 +/- ##
=============================================
- Coverage 70.42% 70.38% -0.05%
=============================================
Files 594 595 +1
Lines 25140 25198 +58
Branches 5676 5689 +13
=============================================
+ Hits 17706 17736 +30
- Misses 7434 7462 +28 |
There was a problem hiding this comment.
Two minor issues. Also I've merged the latest version of the clean-entity-service branch and it now breaks the core issue again (app link fails to update following routes change). Let me know if you want me to investigate following the fix for the other two points.
|
|
||
| @Effect({ dispatch: false }) upateSummary$ = this.actions$.ofType<APISuccessOrFailedAction>(ASSIGN_ROUTE_SUCCESS).pipe( | ||
| map(action => { | ||
| this.store.dispatch(new GetAppSummaryAction(action.apiAction.guid, action.apiAction.endpointGuid)); |
There was a problem hiding this comment.
Instead of an effect here, couldn't we do the same thing we do for the routeReducer and ASSIGN_ROUTE_SUCCESS? Would remove the need for a fresh request and matches the existing pattern
| @@ -62,9 +62,9 @@ export class DeleteRoute extends BaseRouteAction { | |||
| constructor( | |||
| public guid: string, | |||
| public endpointGuid: string, | |||
| appGuid?: string, | |||
There was a problem hiding this comment.
app guid now shouldn't be optional, as it's needed by the app summary reducer. It's not supplied in the space routes list delete action.
irfanhabib
added
comments-addressed
and removed
blocked-pr
needs attention
|
@richard-cox Fixed the two issues |
irfanhabib
added
comments-addressed
and removed
needs attention
Fixes #2518
Blocked on #2551