Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Check version compatibility on login - Closes #309 #314

Merged
merged 6 commits into from
Jun 6, 2017

Conversation

slaweet
Copy link
Contributor

@slaweet slaweet commented Jun 2, 2017

No description provided.

@slaweet slaweet self-assigned this Jun 2, 2017
@slaweet slaweet requested a review from reyraa June 2, 2017 10:42
@slaweet
Copy link
Contributor Author

slaweet commented Jun 2, 2017

To test this you can modify Lisk core node file api/http/loader.js
Comment out

26     'get /status': 'status',

And add:

+  30   router.get('/status', function (req, res) {
+  31     return res.status(404).json({success: false});
+  32   });

*/
check() {
check(notifyUser) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes perfect sense to notify the user of any error which happens during connections. so this parameter seems to be redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that this should be only on login. After login when this fails, the whole ui get's grey and loading spinner is displayed, so the user is notified in either case. I changed the param name accordingly.

@slaweet slaweet force-pushed the 309-version-compatibility-check branch from ea40ce4 to 02ac40a Compare June 3, 2017 07:35
@reyraa
Copy link
Contributor

reyraa commented Jun 6, 2017

Except the typo, all good. thank you

@slaweet slaweet merged commit 4b62fb7 into development Jun 6, 2017
@slaweet slaweet deleted the 309-version-compatibility-check branch June 6, 2017 09:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants