Router issue with multiple routes for the same page with different conditions

[axos88]

This currenty throws a RuntimeException(Unspecified action for page Dashboard at Path(/)) if the user is logged in:

    val loggedInRoutes = emptyRule |
      staticRoute("/", Page.Dashboard) ~> render(Dashboard())

    val routes = emptyRule |
      staticRoute("/login", Page.Login) ~> render(Login()) |
      (staticRoute("/", Page.Dashboard) ~> redirectToPage(Page.Login)(Redirect.Push))
        .addCondition(_ => CallbackTo { !Authentication.isLoggedIn })(_ => None) |
      loggedInRoutes.addCondition(_ => CallbackTo { Authentication.isLoggedIn })(_ =>
        redirectToPage(Page.Login)(Redirect.Push))

    routes.notFound(redirectToPage(Page.Dashboard)(Redirect.Replace)).renderWith(layout)

What I would like to achieve is the user to be redirected to the /login page if he tries to access a protected route without being logged in, and to be redirected to / if he tries to access a non-existent one. Since / is also a protected route currently, it should redirect to /login if the user is not authenticated.

Seems like the system gets confused by having to rules for the same Page with different conditions, which are mutually exclusive, so one of them should ALWAYS trigger. This shouldn't be a routing error.

[japgolly]

Just having a quick look here but it seems to me that if you remove the second definition of staticRoute("/", Page.Dashboard) it will work the way you expect. So:

    val loggedInRoutes = emptyRule |
      staticRoute("/", Page.Dashboard) ~> render(Dashboard())

    val routes = emptyRule |
      staticRoute("/login", Page.Login) ~> render(Login()) |
      loggedInRoutes.addCondition(_ => CallbackTo { Authentication.isLoggedIn })(_ =>
        redirectToPage(Page.Login)(Redirect.Push))

    routes.notFound(redirectToPage(Page.Dashboard)(Redirect.Replace)).renderWith(layout)

[axos88]

Hmm, yeah, seems like it, I'll check this out.
But in general having this kind of setup is not supported? (I mean having the same route with multiple actions based on some mutually exclusive conditions that cover the whole probability space?).

[japgolly]

To be honest, I think that might be undefined behaviour. I don't remember. But I definitely think that if it doesn't fail early saying "hey multiple definitions of the same route aren't supported" then it should work as your expecting.

…

On Thu., 16 May 2019, 11:14 pm Akos Vandra, ***@***.***> wrote: Hmm, yeah, seems like it, I'll check this out. But in general having this kind of setup is not supported? (I mean having the same route with multiple actions based on some mutually exclusive conditions that cover the whole probability space?). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#549?email_source=notifications&email_token=AABRRN42JM7RUGXB7THSBDDPVVM27A5CNFSM4HNC6J3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVRYSKI#issuecomment-493062441>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABRRN5DOLXWSKZLFRS2KPDPVVM27ANCNFSM4HNC6J3A> .

[axos88]

By failing early what do you mean? compile time, or runtime? The runtime exception that occurs, doesn't really satisfy the early failing though, so I guess this is either a bug that needs fixed / feature that has to be added, or a better error message added.

[japgolly]

Hey, FYI, for personal reasons I'm not finding myself with much time to spend on OSS lately. I hope that will get a bit better soon but I do get time I've already got a lot of other stuff to do (esp. Scala 2.13 upgrades) so I don't see myself being able to prioritise this issue for a long while. I do try to give PRs higher priority (esp when they're complete) so that's probably your best bet at helping get this resolved anytime soon.

[axos88]

Gotcha!

[japgolly]

Closed via #616