Refreshing 'Tables' View

[dumbNickname]

Together with @szendo we faced some weird application behavior. Steps to reproduce:

1. Login as cook,
2. Navigate to tables view, should be shown properly
3. Refresh - F5, you will be prompted to login because of:
   POST http://localhost:9000/services/rest/tablemanagement/v1/table/search 403 (Forbidden)
   It looks like this behavior is cased by requests execution order:
   
   As picture shows, search is executed before CSRF token is obtained.
4. If you click cancel, you can continue browsing through the app, but if you try to login again, then you will encounter 'Authentication failed.' message. In order to login again now, browser cache needs to be cleaned.

[lparczew]

I've verified this issue and indeed it is caused by request execution order. Seems like it concerns only the tablemanagement module. At least I was able to reproduce this issue only on this page.
The story is following : Once the page gets refreshed (F5) , the security module runs checkIfUserIsLoggedInAndIfSoReinitializeAppContext() function which is aimed to fully reinitialize the appcontext (including user profile and CSRF token). Unfortunately, the tableMgmt.search state in app.table-mgmt module gets resolved before both userProfile and CRSF token are obtained.

And solution: Add appContext.currentUser promise into chain requesting tableManagement related data (getPaginatedTables function). This promise gets resolved by checkIfUserIsLoggedInAndIfSoReinitializeAppContext(). One of the drawbacks is that each promise in resolve block in stateProvider requesting data from server has to first ensure that appContext is fully initialized which obviously results in code duplication. Maybe there is a way to centralize shared code.

I already implemented above approach : see #70 in pull requests.

If you can come up with more sophisticated approach let me know.

[dumbNickname]

Hi,

First of all - Thanks for sharing your thoughts.

The problem might have affected only the tables module, but only because the lack of complexity in our sample app (besides we do not have lots of modules right now). I would definitely try to go with a general solution here. I have shortly reviewed the pull request: #70 and I would have some small remarks. Imho, putting promise chaining inside the 'tables facotry' seems to be an overkill and a kind of a patch work. There is no relation between table data itself and the user data (or even the refresh scenario). I would say that 'tables factory' does not require the knowledge about appContext.
Besides diagnosing the symptoms I would encourage to dive dipper to understand the problem grounding and then reconsider the fix. I would suggest to quickly go through ui-router implementation details:

• ui router listens on $locationChangeSuccess event - please check urlRouter
• in order to see how resolving works, please have a look at resolve implementation

as it might help to recognize where all the fun takes place during a digest cycle.

Please do not get me wrong, I appreciate the time that you spent on analysis but in my opinion we need a solution that does not require so much attention of the developer and does not introduce superfluous dependencies for factories.

I admit that I do not have a proposal from the top of my head, I will try to have a closer look and think it through as soon as I find some empty miles/spare time. I have some ideas, but don`t know yet if those are even reasonable - need to open the code/debugger.

Maybe other team members will have some proposals, if not - I will definitely come back to you.

Thanks again for your message,
Cheers,
Bartek

[lparczew]

Hi,
thanks a lot for your suggestions. I fully agree with them. Meanwhile, more generic solution came to my mind. I just need a time to test it.

[lparczew]

I've just committed solution which more or less follows dumbNickname suggestions. I did a research and findings are as following. Typically, there are two possible options to overcome such an issue.

• Using $stateChangeStart ( there is also $locationChangeSuccess event but based on the opinions it does not work appropriately in every cases) and this solution has been applied.
• Using abstract States to provide resolved dependencies via resolve for use by child states. In other words, the child state (in our case tableMgmt.search) is waiting until the parent(abstract) state gets resolved (appContext object). As a matter of fact , we don't need to have appContext object resolved in order to use it directly in child states but only to ensure that it gets resolved before child states gets executed. Although, it is not used directly, the child states need the parent state as a dependency (must be injected) in order for it to wait to load ( at least in this particular case I consider this as a some kind of trick).

Both approaches enable us to prevent angular stateProvider from being executed before userProfile gets fully loaded but the first option seems to be more elegant at least for me and does not require developers to have a knowledge about the trick with injection of the resolved dependency between parent <-> child states.

[dumbNickname]

Hi,

i like this approach much more! (even though I am not a big fan of angular events) Thanks for following the suggestion :). I gave $locationChangeSuccess as an idea because it is independent from the router implementation and $stateChangeStart is coupled with ui-router. When I checked ui-router implementation, it is internally based on mentioned $location events as well. But if you found some disadvantages of that approach, the solution with $stateChangeStart is imho fully satisfying.

One really small remark - I would consider putting event handling:

 $rootScope.$on('$stateChangeStart', function (event, toState, toParams) {
            if (bypass) {
                return;
            }
            event.preventDefault();
            appContext.getCurrentUser().then(function () {
                bypass = true;
                $state.go(toState, toParams);
            });
        });

into one of more generic modules, as this behavior is not only targeted at tables module - offers, sales modules need that as well. I know that end effect is the same, but I would consider this problem as a general one and so I would handle it in a place that fits general use cases. Besides it would be nice to add a small test here : )

Thanks again for investigating the topic!

Cheers,
Bartek

[lparczew]

Hi Bartek,
Indeed, at first glance it looks like common logic which can be put into the generic module e.g. main module. It was my first idea as well. Unfortunately, I debugged a bit and it turned out that also other state transitions react on this event, for instance signIn or ''. Obviously, it is appropriate behavior from the point of view of the event implementation but not fully expected in our case ( we don't want simply event to be executed for each state transition). Of course, there is a some kind of "workaround". We can just put some condition with the list of states/locations to prevent event to be triggered/executed. Firstly, it implies that once the new state is added mentioned condition has to be modified as well which leads to unwanted links among modules. Secondly, if it is not needed I don't want event to be even triggered. Lastly, currently I am not aware yet how the authorization concept is implemented and how it should be used in the logic but as far as I know stateChangeStart sometimes is also used for this purposes.
To sum up, I would propose to leave it as it is now and improve it if someone comes up with better idea.

[maybeec]

So this issue can be closed and a new one should be opened with a precise problem description to be solved.

[tomaszwawrzyniakit]

I solved this bug in usersHavingAnyRoleOf method.
I use promise from getUserProfile for all functions in state definition.
It works and is universal.