Merge sessions into integration🎉🎊 #19

helloqx · 2017-03-11T16:48:22Z

Finally, issue #12 is resolved (hopefully) by our very own handler🎉 .

Some points:

I deployed it to test so you can test on the live site
OK linted
Clean local database before pulling
Heroku Timezone is probably US, cleaning of old sessions might be impacted if the heroku postgres timezone != Heroku Timezone (to note)
Consider when to clean old sessions: Database is currently cleaned with every successful login. Maybe clean on logout?
Validation of user done on every single page - might be laggy 😢 - to consider
Small visual bug in which registering a new account shows the base.html because outcome page is used

Commit for common code base

Commented out attempt to use native functions

using GQ ctx version over overwriting sessions handler Conflicts: app.py components/handlers/index.py components/handlers/login.py

…dded basic session authentication:

could login by running 'document.cookie="user=<any value>" in console

users could re-access a logged out account by entering 'document.cookie="user=<logged out user>' before the tab was closed.

nlzz22 · 2017-03-11T18:18:25Z

@helloqx
Note: The live Heroku version crashes when I click on Modified Modules

helloqx · 2017-03-12T01:15:56Z

@nlzz22 oh ya one thing i forgot to note: modified modules doesnt have test cases
I fixed it but havent deployed the new version yet

tgqiang · 2017-03-12T02:54:33Z

@helloqx Test cases for UI?

helloqx · 2017-03-12T03:11:55Z

For not crashing (root.status == 200)
Cause all nose tests passed

tgqiang · 2017-03-12T03:13:51Z

@helloqx I was about to write the UI test cases after checking if Modified Modules needs any more changes for UI side. You have a base test case for that already?

helloqx · 2017-03-12T03:21:05Z

I only have a 3 lines status == 200 test but haven't pushed, just write over ba

Conflicts: app.py components/handlers/outcome.py components/model.py

…n from html handler

helloqx · 2017-03-12T14:30:20Z

@nlzz22 @a0129998
okey pulled latest integration, ready for new review

a0129998 · 2017-03-12T15:14:48Z

components/handlers/module_overview.py

            raise web.seeother('/login')
        input_data = web.input()
        module_code = input_data.code
        module_info = model.get_module(module_code)
+        if module_info is None:


this means that it can't find the module right, so it's taking care of the instance where one user deletes a module and another user tries to view it at the same time?

Yes, no.
It prevents the site from crashing when user accesses wrong module code

a0129998 · 2017-03-12T15:21:51Z

components/handlers/module_view_in_ay_sem.py

            raise web.seeother('/login')

        input_data = web.input()
        module_code = input_data.code
        module_info = model.get_module(module_code)
+        if module_info is None:


the 404 page is used in a few pages, would adding an error message be a good idea? is it going to be used in the final production?

No, 404 is a temporary page

nlzz22 · 2017-03-12T15:26:25Z

test/test_view_individual_module.py

@@ -3,7 +3,8 @@
 '''
 from paste.fixture import TestApp
 from nose.tools import assert_equal, raises


unused raises

nlzz22 · 2017-03-12T15:44:02Z

components/session.py

+    delete_dummy_user_for_tests()
+    create_dummy_user_for_tests()
+    login_session_for_tests(test_app)
+


please use 2 newlines between functions

a0129998 · 2017-03-12T15:56:52Z

test/test_login_registration.py

-        assert_equal(redirected.status, 200)
+        # checks if Validating page loaded
+        response.mustcontain(self.VALIDATING_TITLE)
+        assert_equal(response.status, 200)



why is here a assert_equal(response.status, 200) instead of the assert_equal(response.status, 303) we had previously?

Outcome() is loaded now instead of redirecting back to /login with error code

nlzz22

All files are linted as per comments given, passed all test cases. Session functions as intended.

a0129998 · 2017-03-12T16:18:51Z

All automated testcases passed, no errors found on deployed version through manual testing. sessions functions as intended. Pending test: 2 more more people logging in at the same time.

a0129998 · 2017-03-12T16:30:14Z

2 or more people logging in testing done. all behavior as expected. Pull request approved.

a0129998

All automated testcases passed, no errors found on deployed version through manual testing. sessions functions as intended.

helloqx and others added 30 commits March 1, 2017 15:53

Trying to fix sessions bug

78bb66c

Attempt to generate own sessions handler,

1d20df3

Commit for common code base

Attempt to manually overwrite session files

f7f6895

Commented out attempt to use native functions

Added logout functionality

bc00cf4

Fix for sessions by using web.ctx

14c2c47

Fix for sessions by using web.ctx

1781c55

Added logout routing and session-hook for application

a932c1d

Added logout button

6531ff0

Merge branch 'sessionDebugGq' into sessions-bugfix,

14610ff

using GQ ctx version over overwriting sessions handler Conflicts: app.py components/handlers/index.py components/handlers/login.py

Renamed 'id' -> 'loginStatus' for clarity

966ee9f

Removed unused SESSION import

cef0799

Added userId field to session, for authentication purposes

389bf4f

Minor bugfix with un-renamed field

46d9a29

Renamed ses.py to session.py for clarity, removed unnecessary code, a…

11003c0

…dded basic session authentication:

Changed index session authentication, previously users

cd86824

could login by running 'document.cookie="user=<any value>" in console

Added logout code for sessions, previously

e9de019

users could re-access a logged out account by entering 'document.cookie="user=<logged out user>' before the tab was closed.

Removed debug statement, added exception handling for test cases

e1d61b7

Changed login authentication on login page

ccac7cb

Changed login_registration test to work with new sessions

5dd3c96

Changed login validation method on handler files

a77fe9d

Removed unused SESSION import; changed to new session

f15b2f2

Added methods to support test cases

274ab0d

Fixed test_app.py to pass with new sessions

e6bf23c

Bugfix: Log in with new dummy user, instead of old defunct one

6a66dc1

Fixed test_view_module.py for new sessions

05f15c2

Fixed individual module view for new sessions

445527d

Fixed module edit for new sessions

e9b1b2e

Fixed edit_module_mounting fixture test for new sessions

b104cf5

Fixed test_module_mounting_fixed for new sessions

a75d735

Removed unused web import

799a917

Mass linting of several files

44d265a

helloqx requested review from a0129998 and nlzz22 March 11, 2017 17:10

helloqx added 3 commits March 12, 2017 21:47

Merge branch 'integration' into sessions-bugfix

1cdedf9

Conflicts: app.py components/handlers/outcome.py components/model.py

Removed reference to old SESSION variable

610f2cd

Adjusted test case to use new sessions, removed unnecessary validatio…

148c218

…n from html handler

helloqx temporarily deployed to csmodify March 12, 2017 14:48 Inactive

a0129998 reviewed Mar 12, 2017

View reviewed changes

nlzz22 reviewed Mar 12, 2017

View reviewed changes

helloqx added 2 commits March 12, 2017 23:50

Removed unused raise

239a2db

Added a newline for consistency

c6d279f

a0129998 reviewed Mar 12, 2017

View reviewed changes

nlzz22 approved these changes Mar 12, 2017

View reviewed changes

a0129998 closed this Mar 12, 2017

a0129998 reopened this Mar 12, 2017

a0129998 approved these changes Mar 12, 2017

View reviewed changes

nlzz22 merged commit 6567727 into integration Mar 12, 2017

nlzz22 deleted the sessions-bugfix branch March 12, 2017 17:17

nlzz22 mentioned this pull request Mar 12, 2017

BUG: Sessions are shared across all users #12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge sessions into integration🎉🎊 #19

Merge sessions into integration🎉🎊 #19

helloqx commented Mar 11, 2017 •

edited

Loading

nlzz22 commented Mar 11, 2017

helloqx commented Mar 12, 2017

tgqiang commented Mar 12, 2017

helloqx commented Mar 12, 2017 •

edited

Loading

tgqiang commented Mar 12, 2017

helloqx commented Mar 12, 2017

helloqx commented Mar 12, 2017

a0129998 Mar 12, 2017

helloqx Mar 12, 2017

a0129998 Mar 12, 2017

helloqx Mar 12, 2017

nlzz22 Mar 12, 2017

nlzz22 Mar 12, 2017

a0129998 Mar 12, 2017

helloqx Mar 12, 2017

nlzz22 left a comment

a0129998 commented Mar 12, 2017

a0129998 commented Mar 12, 2017

a0129998 left a comment

Merge sessions into integration🎉🎊 #19

Merge sessions into integration🎉🎊 #19

Conversation

helloqx commented Mar 11, 2017 • edited Loading

nlzz22 commented Mar 11, 2017

helloqx commented Mar 12, 2017

tgqiang commented Mar 12, 2017

helloqx commented Mar 12, 2017 • edited Loading

tgqiang commented Mar 12, 2017

helloqx commented Mar 12, 2017

helloqx commented Mar 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlzz22 left a comment

Choose a reason for hiding this comment

a0129998 commented Mar 12, 2017

a0129998 commented Mar 12, 2017

a0129998 left a comment

Choose a reason for hiding this comment

helloqx commented Mar 11, 2017 •

edited

Loading

helloqx commented Mar 12, 2017 •

edited

Loading