Refractors UrlInterpolationService to not return null at interpolateUrl #13371
Conversation
some title
merged-with-develop
|
Unassigning @EeshaArif since they have already approved the PR. |
|
Assigning @nithusha21, @DubeySandeep for code owner reviews. Thanks! |
|
@nithusha21 @DubeySandeep PTAL |
|
Unassigning @nithusha21 since they have already approved the PR. |
There was a problem hiding this comment.
@mridul-netizen Thanks a lot for this fix, I've left one minor comment, PTAL!
| if (!urlTemplate) { | ||
| this.alertsService.fatalWarning( | ||
| 'Invalid or empty URL template passed in: \'' + urlTemplate + '\''); | ||
| return null; |
There was a problem hiding this comment.
Are we expecting to stop the function execution here?
There was a problem hiding this comment.
Sorry I missed the above comment.
There was a problem hiding this comment.
Can you please add a code comment explaining the same. ALso, can you confirm that we have a unit test to ensure that the function execution stops after line 121?
There was a problem hiding this comment.
I updated the comment. Yes, we're expecting an error, particular unit-tests are there for each case. Actually previously when we were not hitting return null statements then coverage was not full
Removing them also increased the coverage
|
Unassigning @DubeySandeep since the review is done. |
|
@DubeySandeep PTAL! |
|
Unassigning @DubeySandeep since they have already approved the PR. |
|
Hi @mridul-netizen, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks! |
Overview
Essential Checklist
PR Pointers