feat(router): add UrlSegment[] to CanLoad interface #13127

marcuskrahl · 2016-11-29T10:11:06Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

The Route object as passed to implementations of CanLoad which only provides minimal information on the page which should be navigated to. (see #12411)

What is the new behavior?

Additionally to Route an UrlSegment[] is passed to implementations of CanLoad as a second parameter. It contains the array of path elements the user tried to navigate to before canLoad is evaluated.

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

Other information:

CanLoad now defines UrlSegment[] as a second parameter of the function.
Users can store the initial url segments and refer to them later, e.g. to go
back to the original url after authentication via router.navigate(urlSegments).
Existing code still works as before because the second function parameter
does not have to be defined.

Closes #12411

jhuntoo · 2017-05-13T12:21:38Z

@vicb any chance of getting this one merged soon ? It solves a valid use-case (capturing the url without having to load a lazy module), and it appears to be a non breaking change.

jasonaden

Looks good overall. Please make a couple minor updates, rebase, and let's get this merged. Sorry it's been sitting for a while.

jasonaden · 2017-06-06T20:25:59Z

modules/@angular/router/src/apply_redirects.ts

@@ -273,11 +273,12 @@ class ApplyRedirects {
    }
  }

-  private getChildConfig(injector: Injector, route: Route): Observable<LoadedRouterConfig> {
+  private getChildConfig(injector: Injector, route: Route, urlSegments: UrlSegment[]):


For consistency with other methods receiving UrlSegment[], please name the param segments.

jasonaden · 2017-06-06T20:26:29Z

modules/@angular/router/src/apply_redirects.ts

@@ -383,15 +384,16 @@ class ApplyRedirects {
  }
 }

-function runGuards(injector: Injector, route: Route): Observable<boolean> {
+function runGuards(
+    injector: Injector, route: Route, urlSegments: UrlSegment[]): Observable<boolean> {


Name it segments.

jasonaden · 2017-06-06T20:27:24Z

modules/@angular/router/src/interfaces.ts

- *     route: Route
- *   ): Observable<boolean>|Promise<boolean>|boolean {
- *     return this.permissions.canLoadChildren(this.currentUser, route);
+ *   canLoad(route: Route, urlSegments: UrlSegment[]): Observable<boolean>|Promise<boolean>|boolean


This formatting looks bad. Can you reformat more similarly to how it was before?

jasonaden · 2017-07-16T21:54:12Z

@marcuskrahl I went ahead and tried to get this rebased for you. But it's not working after the changes I made. Basically, when you submitted this PR all the files were under modules/@angular/router and now they are under packages/router. Here is the branch where I rebased. If you want this commit, take it. Otherwise, can you get this rebased and sent back in?

googlebot · 2017-07-19T14:44:48Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-07-19T15:12:48Z

CLAs look good, thanks!

googlebot · 2017-07-19T15:21:09Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

googlebot · 2017-07-19T15:30:28Z

CLAs look good, thanks!

marcuskrahl · 2017-07-20T06:48:19Z

@jasonaden I made the necessary changes and everything should work as expected again. The unit tests run successfully. However the travis e2e test seems to be broken for everyone for now so I am waiting for this to be fixed.

jasonaden · 2017-07-26T17:33:47Z

@marcuskrahl Okay, thanks.

Please rebase again. We had an issue that messed up the commit histories on many PRs. So this one needs a new rebase as it's showing hundreds of commits.

ghost · 2017-10-11T13:57:00Z

What's the status of this?

MickL · 2017-10-20T10:50:24Z

Please merge

picosam · 2017-12-11T19:11:03Z

Any idea about when this should be merged?

marcuskrahl · 2018-01-14T09:56:05Z

I rebased again. Giving this another try to get merged

digaus · 2018-02-06T13:33:13Z

When will it be merged?

ngbot · 2018-03-20T20:58:20Z

Hi @marcuskrahl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

ngbot · 2018-03-20T20:58:42Z

Hi @marcuskrahl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

CanLoad now defines UrlSegment[] as a second parameter of the function. Users can store the initial url segments and refer to them later, e.g. to go back to the original url after authentication via router.navigate(urlSegments). Existing code still works as before because the second function parameter does not have to be defined. Closes angular#12411

marcuskrahl · 2018-03-29T12:27:04Z

I fixed the conflicts so this PR should be good to merge again

PoiScript · 2018-04-18T12:03:54Z

Any news? I'm hoping that this can be merged and included in the v6.

ratidzidziguri · 2018-05-22T19:32:46Z

Any update on this? we also need this fix to be available.

ngbot · 2018-08-16T23:25:24Z

I see that you just added the PR action: merge label, but the following checks are still failing:
     missing required label: "PR target: *"
     status "code-review/pullapprove" is failing
     status "google3" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

CanLoad now defines UrlSegment[] as a second parameter of the function. Users can store the initial url segments and refer to them later, e.g. to go back to the original url after authentication via router.navigate(urlSegments). Existing code still works as before because the second function parameter does not have to be defined. Closes angular#12411 PR Close angular#13127

angular-automatic-lock-bot · 2019-09-13T23:23:01Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Nov 29, 2016

vicb added area: router action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 9, 2016

jasonaden approved these changes Jun 6, 2017

View reviewed changes

jasonaden added the action: discuss label Jul 17, 2017

googlebot added cla: no and removed cla: yes labels Jul 19, 2017

marcuskrahl force-pushed the add-url-segments-to-canload branch from 62afa5e to 312f397 Compare July 19, 2017 15:12

googlebot added cla: yes and removed cla: no labels Jul 19, 2017

googlebot added cla: no and removed cla: yes labels Jul 19, 2017

marcuskrahl force-pushed the add-url-segments-to-canload branch from 84292f3 to 92e4eeb Compare July 19, 2017 15:30

googlebot added cla: yes and removed cla: no labels Jul 19, 2017

marcuskrahl force-pushed the add-url-segments-to-canload branch from 0dd38d2 to cf31338 Compare July 19, 2017 18:49

mhevery force-pushed the master branch from 08b36cb to 5d4b36f Compare July 20, 2017 21:48

marcuskrahl force-pushed the add-url-segments-to-canload branch from cf31338 to 22faaf3 Compare August 2, 2017 19:55

marcuskrahl force-pushed the add-url-segments-to-canload branch from 22faaf3 to 9141447 Compare January 14, 2018 08:08

marcuskrahl force-pushed the add-url-segments-to-canload branch from 9141447 to fc68e9a Compare March 29, 2018 10:09

marcuskrahl force-pushed the add-url-segments-to-canload branch from fc68e9a to 68ba3d7 Compare March 29, 2018 11:24

jasonaden self-assigned this Jun 19, 2018

jasonaden added the jira-sync label Jul 25, 2018

jasonaden added action: merge The PR is ready for merge by the caretaker and removed action: discuss action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 16, 2018

jasonaden added the target: major This PR is targeted for the next major release label Aug 16, 2018

jasonaden closed this in 07d8d39 Aug 16, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): add UrlSegment[] to CanLoad interface #13127

feat(router): add UrlSegment[] to CanLoad interface #13127

marcuskrahl commented Nov 29, 2016 •

edited by jasonaden

Loading

jhuntoo commented May 13, 2017

jasonaden left a comment

jasonaden Jun 6, 2017

jasonaden Jun 6, 2017

jasonaden Jun 6, 2017

jasonaden commented Jul 16, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

marcuskrahl commented Jul 20, 2017

jasonaden commented Jul 26, 2017

ghost commented Oct 11, 2017

MickL commented Oct 20, 2017

picosam commented Dec 11, 2017

marcuskrahl commented Jan 14, 2018

digaus commented Feb 6, 2018

ngbot bot commented Mar 20, 2018

ngbot bot commented Mar 20, 2018

marcuskrahl commented Mar 29, 2018

PoiScript commented Apr 18, 2018

ratidzidziguri commented May 22, 2018

ngbot bot commented Aug 16, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

feat(router): add UrlSegment[] to CanLoad interface #13127

feat(router): add UrlSegment[] to CanLoad interface #13127

Conversation

marcuskrahl commented Nov 29, 2016 • edited by jasonaden Loading

jhuntoo commented May 13, 2017

jasonaden left a comment

Choose a reason for hiding this comment

jasonaden Jun 6, 2017

Choose a reason for hiding this comment

jasonaden Jun 6, 2017

Choose a reason for hiding this comment

jasonaden Jun 6, 2017

Choose a reason for hiding this comment

jasonaden commented Jul 16, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

googlebot commented Jul 19, 2017

marcuskrahl commented Jul 20, 2017

jasonaden commented Jul 26, 2017

ghost commented Oct 11, 2017

MickL commented Oct 20, 2017

picosam commented Dec 11, 2017

marcuskrahl commented Jan 14, 2018

digaus commented Feb 6, 2018

ngbot bot commented Mar 20, 2018

ngbot bot commented Mar 20, 2018

marcuskrahl commented Mar 29, 2018

PoiScript commented Apr 18, 2018

ratidzidziguri commented May 22, 2018

ngbot bot commented Aug 16, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

marcuskrahl commented Nov 29, 2016 •

edited by jasonaden

Loading