veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
201 stars 30 forks source link

Base group call to redirect() doesn't cancel call to extended group waitOn() #89

Closed drone1 closed 2 years ago

drone1 commented 3 years ago

I'm having an issue:

I've got a nested/child group, and although the base group redirects, the child group's waitOn() gets called. Is this expected? This is best illustrated with the code.

export const loggedIn = FlowRouter.group({
    triggersEnter: [
        (context, redirect) => {
            if (!Meteor.user()) {
                redirect('/login')
                return
            }
        }
    ]
});

export const adminRoutes = loggedIn.group({
    name: "adminRoutes",
    prefix: "/admin",
    waitOn(params, query, ready) {
        throw `Why is this being called?`
    },
})

adminRoutes.route('/', {
    action(params, query, data) { /* ... */ }
})

As you can see, one can't rely on Meteor.user() because redirect() call in loggedIn.triggersEnter doesn't cancel call to adminRoutes.waitOn().

Thanks!

ostrio:flow-router-extra@v3.7.5 (ostrio:flow-router-meta@2.1.1) Meteor@2.1 Chrome@91.0.4472.77 (Official Build) (arm64) Mac Book Pro, Big Sur@11.3.1

dr-dimitru commented 2 years ago

Hello @drone1 ,

There's no way to alter this behavior, method called triggersEnter, not triggersBeforeEnter, meaning that triggers actually called after waitOn, since by architecture you may need to pre-fetch data in waitOn to use it later in triggersEnter hooks

Hope that helps. Feel free to reopen it in case if the issue is still persists on your end.

P.S. I'm sorry for the late reply

drone1 commented 2 years ago

Hello @drone1 ,

There's no way to alter this behavior, method called triggersEnter, not triggersBeforeEnter, meaning that triggers actually called after waitOn, since by architecture you may need to pre-fetch data in waitOn to use it later in triggersEnter hooks

Hope that helps.

Feel free to reopen it in case if the issue is still persists on your end.

P.S. I'm sorry for the late reply

Hi, thanks.

I don't see triggersBeforeEnter so how is one supposed to implement this kind of flow in a clean way? Suggestions would be appreciated.

The docs show an example that does not demonstrate how a super user is actually verified or differentiated from an admin in the "nested groups" example. A redirect or error is necessary. How does one implement this? Thank you!

dr-dimitru commented 2 years ago

I don't see triggersBeforeEnter so how is one supposed to implement this kind of flow in a clean way? Suggestions would be appreciated.

There's no such function, that's what I meant.

Try next approach:

const isAdmin = () => {
  // verify admin user permissions
  // You can return promise and waitOn for Method response
};

const adminGroup = FlowRouter.group({
  prefix: '/admins',
  name: 'admin',
  title: 'Admin Panel',
  meta: {
    robots: 'noindex, nofollow'
  }
});

adminGroup.route('/', {
  name: 'adminHome',
  waitOn() {
    if (isAdmin()) {
      return import('/imports/client/Admin/index/index.js');
    }
  },
  action() {
    if (isAdmin()) {
      this.render('layout', 'adminHome');
    } else {
      this.render('notfound404');
    }
  }
});

Let me know if this helps

drone1 commented 2 years ago

Thanks, but then how do you implement a logged-in group with a nested admin group?

It seems there is no way for the logged-in group to redirect, (which is what you want, not an error page because the user isn’t logged in), and from what you’re saying it seems like one would need to duplicate logic in both groups, which defeats the purpose of nested groups, no? The admin group should not need to check if the user is logged in IMO.

Thanks for your help!

On Fri 3. Jun 2022 at 17:51, dr.dimitru @.***> wrote:

I don't see triggersBeforeEnter so how is one supposed to implement this kind of flow in a clean way? Suggestions would be appreciated.

There's no such function, that's what I meant.

Try next approach:

const isAdmin = () => { // verify admin user permissions // You can return promise and waitOn for Method response}; const adminGroup = FlowRouter.group({ prefix: '/admins', name: 'admin', title: 'Admin Panel', meta: { robots: 'noindex, nofollow' }}); adminGroup.route('/', { name: 'adminHome', waitOn() { if (isAdmin()) { return import('/imports/client/Admin/index/index.js'); } }, action() { if (isAdmin()) { this.render('layout', 'adminHome'); } else { this.render('notfound404'); } }});

Let me know if this helps

— Reply to this email directly, view it on GitHub https://github.com/veliovgroup/flow-router/issues/89#issuecomment-1146097094, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQIEPD7FJ75CRD5ZM672Z3VNISZFANCNFSM46RUZCFQ . You are receiving this because you were mentioned.Message ID: @.***>

dr-dimitru commented 2 years ago

@drone1 to be honest in most of the apps we use Blaze, and implement this logic on the level of the template or "adminLayout" where if user is verified as Admin — pull data and render the template, if not show login or 404 page. Moving data logic to the template level as well.

Hope that helps

drone1 commented 2 years ago

I’m looking to do this at the routing level. Thanks.

On Sat 4. Jun 2022 at 15:12, dr.dimitru @.***> wrote:

@drone1 https://github.com/drone1 to be honest in most of the apps we use Blaze, and implement this logic on the level of the template or "adminLayout" where if user is verified as Admin — pull data and render the template, if not show login or 404 page. Moving data logic to the template level as well.

Hope that helps

— Reply to this email directly, view it on GitHub https://github.com/veliovgroup/flow-router/issues/89#issuecomment-1146607503, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQIEPAUJFVG7HB3VM6F7NDVNNI5LANCNFSM46RUZCFQ . You are receiving this because you were mentioned.Message ID: @.***>