Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User is able to enter data in forms before session expiry flow could execute given connectivity is there #958

Closed
mwkhan1983 opened this issue Apr 16, 2021 · 27 comments · Fixed by #959 or #963
Assignees

Comments

@mwkhan1983
Copy link

Either user should be able to save data entered upon submit or delay should not be long enough to allow user to keep entering data.

@vincent-karuri
Copy link
Contributor

vincent-karuri commented Apr 16, 2021

Another option would be not to logout the user if they are filling out a form. Set the current activity in the onCreate of the CovidJsonFormActivity and clear it onStop.

Then check what activity is currently visible before deciding to logout or not. This may be an easier and more efficient lift.

@owais-vd
Copy link
Contributor

I have checked by setting up the activity in CovidJsonFormActivity but it's not working because there is a process before starting the activity and it takes a time to load and in the meantime logout method called before setting up the activity.

@vincent-karuri
Copy link
Contributor

@syedowaisali90 what process are you referring to?

@vincent-karuri
Copy link
Contributor

what do is meant by it's not working? what about invoking this function makes it not work?

@owais-vd
Copy link
Contributor

it's just processed all the components before starting up the form activity and it takes some time that' why we are showing the loader.

@vincent-karuri
Copy link
Contributor

what exactly is the issue with that? does logout happen after the form is already launched?

@owais-vd
Copy link
Contributor

yes, logout happens after the form is already lunched https://drive.google.com/file/d/15BnnMpFqrg6qdLn0R3Si2eeweMxQGqge/view

@vincent-karuri
Copy link
Contributor

is this after making the change here #958 (comment)? Why would that happen?

@owais-vd
Copy link
Contributor

#958 (comment) this change won't work as i mentioned here #958 (comment)

@vincent-karuri
Copy link
Contributor

the video you shared, #958 (comment) is it after making the change or not?

@owais-vd
Copy link
Contributor

No

@vincent-karuri
Copy link
Contributor

@syedowaisali90 could you provide a detailed description or short video of what happens when you apply the change? I don't currently understand what is meant by it doesn't work.

@owais-vd
Copy link
Contributor

Here is a scenario:

  1. The user has resumed profile activity and at the same time, userAuth has triggered in the background.
  2. Let assume userAuth in progress and meanwhile, a user tried to open a form.
  3. Now showing a form loader.
  4. Before starting the form activity i.e CovidJsonFormActivity. userAuth gets completed.
  5. At that moment in userAuth tried to get a current from RDTApplication class.
  6. But currently we don't have any activity in the RDTApplication class. Because of form launch still in progress.
  7. Now userAuth tried to logout and after that startActivity method get called and launched the form.
  8. When the form launched then we set the current activity in RDTApplication class.
  9. But setting up the activity is useless because userAuth has been already triggered.

@vincent-karuri
Copy link
Contributor

Cool, I still don't see why the service having already started is an issue because if at the point it's calling logout the form activity is already launched/visible, then the form activity would have already registered itself as the current activity and logout should not happen. Unless you're saying logout happens while in 3 (the spinner is showing).

Are you able to reproduce forced logout when the form is showing for this scenario #958 (comment)?

@owais-vd
Copy link
Contributor

I mean this Unless you're saying logout happens while in 3 (the spinner is showing). at this moment we don't have a CovidJsonFormActivity in RDTApplication class and this happened frequently. As you have seen here #958 (comment)

@vincent-karuri
Copy link
Contributor

#958 (comment) shows that logout happens while the form is already launched, not while the spinner is showing.

@owais-vd
Copy link
Contributor

done #959 (comment)

@mwkhan1983
Copy link
Author

@vincent-karuri @syedowaisali90 Please see attached video. Issue is occurring again. User is filling in the data not knowing session has expired and when complete form is filled and submitted, data is not saved as user has already logged out in the background.

The authentication check once the form opens was working well. It was not allowing user to enter data and logging out. Not sure why we changed that.

@mwkhan1983 mwkhan1983 reopened this Apr 30, 2021
@vincent-karuri
Copy link
Contributor

@mwkhan1983 What was merged was supposed to ensure that the app doesn't log out when a form is open . If this is still happening, we need to list the steps to reproduce that behaviour and get it definitively fixed . I'm not sure what was working before that is not working now since that is the bug described in the issue (logging out when filling out a form).

@mwkhan1983
Copy link
Author

@vincent-karuri
App doesn't logout when form is open but when you submit the form, data is not saved because user is already logged out in the background. This is an issue because user would not know that despite submitting data, it is not saved.

So original issue described asked for one of the two things to happen:

  1. App doesn't logout when form is open and saves data on form submission.
  2. App does logout when form is opened but that should not take long enough for user to be able to enter substantial data.

Through authorization check when opening form, we were able to achieve 2.

@vincent-karuri
Copy link
Contributor

  1. I'm not sure what is meant by logged out in the background, could you elaborate?
  2. How was it verified that the data is not saved?
  3. Could you provide steps to reproduce this
  4. How does adding authz when launching a form prevent 2 from occurring? authz is an asynchronous process meaning it allows the form to open while still running in the background (so no guarantees it completes before the form launches). It was removed because it doesn't make too much sense to call this on every form launch (resource intensive).

@mwkhan1983
Copy link
Author

  1. Did you see the video shared in this comment User is able to enter data in forms before session expiry flow could execute given connectivity is there #958 (comment)? When you see it, it will be clear that the new thread starts when form is launched while auth activity runs on another thread. When form thread completes on submitting the form, user is directly taken to a logout screen.
  2. Simple. Login again and see the data submitted. No data found in history
  3. Steps: login. Wait for 20-25 minutes (session timeout). Resume app. Open any of the forms. Complete and submit form. User is taken to login screen on form submission. Login again. Data submitted is not saved and not shown in history.
    All these steps are pretty clear to @syedowaisali90 . He is able to reproduce this issue
  4. After Owais applied auth on forms, I have tested it thoroughly. it was working fine, logging out user with a message that session is expired after from is opened. Auth was checking after form is opened in the same thread. and applying on every form is what makes sense. Because patient view, profile view are read-only. It doesn't make a difference if user is interacting with those screens even if session is timedout. However, when user is interacting with forms entering data, that is where the issue is. User not knowing session is timed-out and keep entering data. At the end when form is submitted, user realize that all data that is entered is lost/not saved because session wasn't there.

@vincent-karuri
Copy link
Contributor

@mwkhan1983 one cannot make the conclusion in 1 by looking at the video. There should be no "background logout" if the current activity is a form activity (has @syedowaisali90 confirmed the "background logout" during debugging?).

I think all steps should ideally be logged on the issue so that they are visible to everyone involved.

Authz has always been done in a background thread. Again, this is not a conclusion that can be made by looking at the video. Even if we revert the authz to happen on every form launch, it's simply coincidental that the authz check happens fast enough before too many details are entered into the form and is not a definitive fix imo.

@syedowaisali90 could you check why logout happens soon after submission, and also explore if this is a problem with the form activity being cleared before submission is complete?

@owais-vd
Copy link
Contributor

owais-vd commented May 3, 2021

@vincent-karuri could you test the app from step 3 here #958 (comment) so that you see the real problem.

@vincent-karuri
Copy link
Contributor

@syedowaisali90, @mwkhan1983 has described what is happening behaviorally. We should try figure out why what is happening in code. There may be other reasons but two things to keep in mind is:

  1. Figure out whether logout is happening because the current activity is cleared and VerifyAuthorizationTask runs when saving a form
  2. Or sync happens when the form is open, triggering forced-logout - we don't check the current activity before triggering sync atm

@owais-vd
Copy link
Contributor

owais-vd commented May 4, 2021

@vincent-karuri Here is the scenario #958 (comment). The main reason, why it's happening just because syncUtils.logoutUser(); get called called during the form loading.

When we try to open a form so it takes some time to open a CovidJsonFormActivity so that's why I have used caller activity as a check instead of CovidJsonFormActivity.

Currently, we have only two caller activity the one is CovidPatientProfileActivity and the second is CovidPatientRegisterActivity and I have tested that now it's working fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment