vue-browser-acl icon indicating copy to clipboard operation
vue-browser-acl copied to clipboard

How to make the directives react on state changes?

Open thapar opened this issue 5 years ago • 12 comments

I am using vue-browser-acl's custom directive as such:

<template>
  <div>
      <BaseSideNavigationMenu v-role:authenticated />
      <PageContent />
  </div>
</template>

With the following setup and rule:

Vue.use(Acl, store.state.currentUser, (acl) => acl.rule("authenticated", () => store.getters.isAuthenticated));

And the following in my store:

export default new Vuex.Store({
  state: {
    currentUser: JSON.parse(window.localStorage.getItem("currentUser") || "{}"),
  },
  mutations: {
    SET_CURRENT_USER(state, user) {
      state.currentUser = user;
      window.localStorage.setItem("currentUser", JSON.stringify(user));
    },
    SET_NO_CURRENT_USER(state) {
      state.currentUser = {};
      window.localStorage.setItem("currentUser", "{}");
    },
  },
  actions: {
    async login({ commit }, login_data) {
      try {
        const response = await axios.post("/api/login", login_data);
        const currentUser = response.data.currentUser
        commit("SET_CURRENT_USER", currentUser);
        return response;
      }
    },
    async logout({ commit }) {
      const response = await axios.post("/api/logout");
      commit("SET_NO_CURRENT_USER");
    },
  },
  getters: {
    isAuthenticated(state) {
      return Object.keys(state.currentUser).length > 0;
    }
  },

However, when a user does a login, the <BaseSideNavigationMenu v-role:authenticated /> element does not show unless the page is refreshed. Likewise, upon logout, the BaseSideNavigationMenu element remains visible until the page is refreshed.

How can vue-browser-acl's directive be made to react on state changes?

thapar avatar Apr 06 '20 14:04 thapar

Check the index file in the example folder.

The user parameter must be a function if it is loaded dynamically.

// Since the user is asynchronously fetch we
// provide a function to fetch it in this case
// we access it through the Vuex store.
const user = () => {
  return store.getters.user
}

Vue.use(Acl, user, (acl) => { ... })

mblarsen avatar Apr 06 '20 16:04 mblarsen

@mblarsen Upon further tinkering, I noticed that certain elements are reactive with v-role:authenticated tacked on, and others are not. I made the following changes, as per your suggestion, but the outcome of which elements are reactive still hasn't changed:

Vue.use(Acl, () => store.getters.currentUser, (acl) => acl.rule("authenticated", () => store.getters.isAuthenticated));

And I updated the getters portion of the store to include the following:

getters: {
    isAuthenticated(state) {
      return Object.keys(state.currentUser).length > 0;
    },
    currentUser(state) {
      return Object.keys(state.currentUser).length > 0 ? state.currentUser : {};
    }
}

The following elements are responsive as expected:

<span v-role:authenticated.not>Login</span>
<span v-role:authenticated>{{ currentUser.name }}</span>

But the following elements are not responsive:

<LoginForm v-role:authenticated.not />
<LoggedinMenu v-role:authenticated />

The full project code can be seen by following the links above or here.

thapar avatar Apr 06 '20 22:04 thapar

Can you please test if this works:

<LoginForm v-if="$can('authenticated')" />

mblarsen avatar Apr 07 '20 03:04 mblarsen

I looked at the repo you shared.

All of the following isn't really about the issue you've raised. Just some observations. Hope you don't mind :)


When you add () => store.getters.currentUser you provide a function that Acl uses to pass as the first parameter to the rules you define.

So your rule should ideally be:

-  acl.rule("authenticated", () => store.getters.isAuthenticated)
+  acl.rule("authenticated", (user) => Boolean(user))

If the user is present you are authenticated. Although your currentUser-getter is returning an "empty" object if the user is not there. Maybe null would be better?

What you've done isn't wrong. Just a bit confusing. They idea of the Acl is to ask "can the current user do x" or "what is true about the current user" - so ideally you should resolve your logic from the user's perspective as I've shown above.

Also:

<LoginForm v-role:authenticated.not />

Is kind of misuse of the semantics. The authenticated isn't really a role. You could perhaps instead create a rule called login.

<LoginForm v-can:login />

But really when it comes to UI flow I'd use the store getters directly in your case:

<LoginForm v-if="!authenticated" />
<LoggedinMenu v-if="authenticated" />

For me it is all about how the code reads. If "authenticated do this if not do that".

mblarsen avatar Apr 07 '20 04:04 mblarsen

Can you please test if this works:

<LoginForm v-if="$can('authenticated')" />

This works reactively as expected. Can the v-role syntax not be reliably used reactively?

thapar avatar Apr 07 '20 04:04 thapar

I looked at the repo you shared.

All of the following isn't really about the issue you've raised. Just some observations. Home you don't mind :)

When you add () => store.getters.currentUser you provide a function that Acl uses to pass as the first parameter to the rules you define.

So your rule should ideally be:

-  acl.rule("authenticated", () => store.getters.isAuthenticated)
+  acl.rule("authenticated", (user) => Boolean(user))

If the user is present you are authenticated. Although your currentUser-getter is returning an "empty" object if the user is not there. Maybe null would be better?

What you've done isn't wrong. Just a bit confusing. They idea of the Acl is to ask "can the current user do x" or "what is true about the current user" - so ideally you should resolve your logic from the user's perspective as I've shown above.

Also:

<LoginForm v-role:authenticated.not />

Is kind of misuse of the semantics. The authenticated isn't really a role. You could perhaps instead create a rule called login.

<LoginForm v-can:login />

But really when it comes to UI flow I'd use the store getters directly in your case:

<LoginForm v-if="!authenticated" />
<LoggedinMenu v-if="authenticated" />

For me it is all about how the code reads. If "authenticated do this if not do that".

"authenticated" is only a simplified example. In the final application, there will be "moderator", "admin", "superadmin", etc. But as per your last example with v-if="authenticated" it seems like you're saying not to use vue-browser-acl 😭. I like the syntax of separating ACL permissions for components and routes from the business logic (the code will have plenty of other v-ifs to deal with that aren't related to ACL permissions). Also, this blogpost makes excellent points for using your vue-browser-acl library (and I like the syntax and flexibility of this library).

thapar avatar Apr 07 '20 04:04 thapar

This works reactively as expected. Can the v-role syntax not be reliably used reactively?

I haven't had any issues with it previously. When you work Vuex and async in general things change a bit. Anyway, I'll have to look more into to your issue. Thanks for testing the $can part.

Can you try this too:

+<div v-if="authenticated || !authenticated">
    <LoginForm v-role:authenticated.not />
    <LoggedinMenu v-role:authenticated />
+</div>

Where authenticated is the getter from your store.

Never mind the logic. This is just to see what happens if it works after being forced to re-render by a parent. This is to exclude the issue being something else.

mblarsen avatar Apr 07 '20 04:04 mblarsen

This works reactively as expected. Can the v-role syntax not be reliably used reactively?

I haven't had any issues with it previously. When you work Vuex and async in general things change a bit. Anyway, I'll have to look more into to your issue. Thanks for testing the $can part.

Can you try this too:

+<div v-if="authenticated || !authenticated">
    <LoginForm v-role:authenticated.not />
    <LoggedinMenu v-role:authenticated />
+</div>

Where authenticated is the getter from your store.

Never mind the logic. This is just to see what happens if it works after being forced to re-render by a parent. This is to exclude the issue being something else.

Yes, it is reactive with the v-if wrapping.

thapar avatar Apr 07 '20 04:04 thapar

Hey @mblarsen, just wondering if you may have had a chance to look into the reactivity issue?

thapar avatar Apr 08 '20 04:04 thapar

Hey @mblarsen, just wondering if you may have had a chance to look into the reactivity issue?

I have not. Don't expect any quick response. I cannot put so much time into it at the moment.

mblarsen avatar Apr 08 '20 04:04 mblarsen

Can you please test if this works:

<LoginForm v-if="$can('authenticated')" />

This works reactively as expected. Can the v-role syntax not be reliably used reactively?

I am having exactly the same issue. Thanks for raising this up and coming to this workaround. I would however also appreciate if the better syntax would work reactively ;-)

josefsabl avatar Sep 17 '20 13:09 josefsabl

Hi @thapar @josefsabl

I've spend a little time updating the example (changes have been pushed).

Would you mind checking it out and try this flow:

  1. Log in as 'jane' in 'admin' group
  2. See the Admin paragraph appear at the top
  3. Log out and see
  4. See the Admin paragraph disappear again

I'm wondering how this (working) example is different from yours. When I log in and out the user object in the state is replace completely. Could that be what makes the difference? That you don't replace your state object.

mblarsen avatar Sep 19 '20 09:09 mblarsen