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

Login states and webservice results to test #313

Closed
AlexanderPico opened this issue Jun 3, 2015 · 19 comments
Closed

Login states and webservice results to test #313

AlexanderPico opened this issue Jun 3, 2015 · 19 comments

Comments

@AlexanderPico
Copy link
Member

Login state

  1. Not logged in -- currently, I can open the editor, make edits and save them while not logged in... though it fails with a 500 status in the responseText. You shouldn't even see the "Quick Edit" button.
  2. Logged in -- common case that we've been testing
  3. Log in expired while editor is open -- need to figure out how to prevent this, ideally. In addition, we should safeguard against it by checking for this condition (with each edit) as part of the logic of whether we show users the "save" button.

Webservice results

  1. 200, saved -- should return a newly minted revision number that pvjs can use
  2. 200, not saved -- show message? offer suggestion or help link?
    a. latest revision number has changed
    b. login state not valid
    c. other reasons?
  3. 500, no response -- show message? offer suggestion or help link?
    a. web services are broken
  4. Offline -- show message? offer suggestion or help link?
@AlexanderPico
Copy link
Member Author

The "bug" aspect of this issue is that anyone can navigate to (or bookmark or remain on) the page with the editor/open or editor/closed anchors, whether they are logged in or not.
http://pvjs.wikipathways.org/index.php/Pathway:WP4#/editor/open

This should not be allowed. When those anchors are called the server should not respond before checking login state. It would be even better if they weren't anchors in the url, so that folks don't send extra info when copy/pasting urls, e.g.:
http://pvjs.wikipathways.org/index.php/Pathway:WP4#/editor/closed
when they just want to share or publish a link to the pathway.

Likewise, people could share this link:
http://pvjs.wikipathways.org/index.php/Pathway:WP4#/editor/disabled
and then no one would be able to edit, even if they were logged in...

We might need a refresh strategy rather than an anchor strategy, unfortunately...

@AlexanderPico
Copy link
Member Author

For login state (3), I have set the following in apache config and php.ini files for wikipathways.org and pvjs.wikipathways.org:

  • php_value session.gc_maxlifetime 604800 // 7 days in seconds
  • php_value session.cookie-lifetime 0 //until browser closed
  • php_value session.cache_expire 10080 // 7 days in minutes

Also set $wgCookieExpiration in LocalSettings.php. Altogether, these should extend login timeouts.

This should cover most cases of (3), though we should still verify the login state before submitting to webservice and notify user that they need to login, rather than sending a webservice call that we know will fail...

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

WRT webservice results, the following are commonly used:

200: update successful; data returned in response (e.g, version number)
204: update successful; no data returned
400: Bad request (e.g., missing parameters)
500: Internal server error

What do you mean by offline?

@AlexanderPico
Copy link
Member Author

Ah, the concern isn't the use of standard numbers. These are already implemented. The issue here is what do we do with each of the responses as a client.

Offline, meaning they lose their connection. The issue then being, what happens to the editor at various stages if disconnected. Any data loss scenarios? Any cases where we need to notify the user?

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

WRT to the URL, we have several options. One question: many users really dislike for web apps to "break" their browser's back button. If a user visits the page with the editor closed, opens the editor, then clicks Back, what does the user expect?

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

For the webservice results, how do we distinguish between 1 and 2?

@AlexanderPico
Copy link
Member Author

1 and 2 are distiguished by parsing the responseText... which is still garbledygook...

@AlexanderPico
Copy link
Member Author

I would guess that some users expect to go to closed and some expect to go previous page :) Would depend on their individual intentions. Either is fine with me.

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

I thought the user would expect the back button to return them to the closed state, and that behavior requires the disabled, open, closed state to be included in the URL. If clicking back should take the user away from the current pathway page, we can get rid of the extra info in the URL.

The initial state of the editor can be controlled by the editor attribute on the custom element, e.g., editor="closed"

@AlexanderPico
Copy link
Member Author

That might be true, but then we need to be able to deal with sharing and publishing of urls that don't match login state. As it is now, the problems outweigh the benefits. I'd rather the "back" button not satisfy the particular user that wants one things than to have urls that create broken states.

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

I'll see whether it's easy to get rid of it.

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

Regarding Login State item 3) from the OP, the most common way of handling this is to return a 401 response status code from the server; then on the front-end, prompt the user to login again so pvjs can retry the save. It could be difficult to try determining from the front-end whether the user's login expired.

@AlexanderPico
Copy link
Member Author

Exactly, things like this are unique challenges to developing within the MediaWiki framework. So, not only might it be difficult to detect login state from the front end, but when you say "prompt the user to login and try again," well, that's just not possible without navigating to another page (the login page) and then navigating back to the pathway page. What state should the editor be in then? What state would the local svg/json/gpml model be in? The "common way" might not work for us; we have to develop WikiPathways-specific solutions.

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

Leaving the pathway page would lose the current in-memory pvjs state. We would have use something like session storage or local storage and/or the special URL, e.g. "...#!/editor/closed".

Is it possible to determine whether the user is logged in from the front-end?

@AlexanderPico
Copy link
Member Author

Good question. Can you make php calls to mediawiki from front-end? In php, it looks like this:

$wgUser->isLoggedIn()

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

Frontend can only access API endpoints, so that wouldn't be directly possible.

@AlexanderPico
Copy link
Member Author

Right. The OP list is not necessarily things I expect we can address (technically or timely). Rather they are a list of things we should test to be aware of so we can mitigate, document and balance impl decisions.

So, like the case for editing currently, the pvjs editor might lose track of login state. Ok. Then what do we do when this happens? Presuming we can detect this state when the web service returns a legible responseText, can we keep the data around and offer a second attempt after they've logged back in? Can we show them a message that explains why saving failed and how to log in and then remake the changes? Where does the message appear? Do we disable the editor? Etc...

For example, perhaps here we could provide a url that references a cached item. When they go to login and return via the mediawiki-provided callback link, then could we bring up the changed state ready to be saved again? Or, we could recommend they open a new window, login there and then try again without leaving pvjs...

@ariutta
Copy link
Member

ariutta commented Jun 4, 2015

Probably possible if we do something along the lines of my earlier comment:

We would have use something like session storage or local storage and/or the special URL, e.g. "...#!/editor/closed".

@khanspers
Copy link
Member

Looks like the part of this issue that was a bug has been fixed. I removed the bug label and added "enhancement" instead.

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

No branches or pull requests

3 participants