S8 signup flow basic profile #213

NunoDasNeves · 2018-10-10T06:48:58Z

Signup flow is basic but pretty robust now.
First you create an account with firebase, then you must create a 'profile' (backend account).

Not added yet, but you need firebase + profile to post questions/reviews etc

Basic profile page exists, and you can update fields.

Reset password added - it uses firebase interface for the actual reset. Doing it on SmartCourse is possible but extra hassle and might not work on localhost.

coveralls · 2018-10-10T06:58:07Z

Pull Request Test Coverage Report for Build 834

0 of 0 changed or added relevant lines in 0 files are covered.
13 unchanged lines in 4 files lost coverage.
Overall coverage increased (+1.3%) to 82.729%

Files with Coverage Reduction	New Missed Lines	%
src/controllers/user.js	2	72.73%
src/models/user.js	3	68.0%
src/utils/helpers.js	3	68.18%
src/models/db/tables.js	5	86.31%

Totals
Change from base Build 829:	1.3%
Covered Lines:	495
Relevant Lines:	553

💛 - Coveralls

DarkPurple141

See comments. Mostly looks good.

backend/src/controllers/user.js

backend/src/models/db/tables.js

backend/src/models/user.js

DarkPurple141 · 2018-10-10T11:04:01Z

frontend/src/store/auth/index.js

+    return createProfile(user, { displayName })
+      .then((profile) => commit('SET_PROFILE', profile, { root: true }))
+      .catch(error => {
+        commit('ERROR', error.message)


This pattern is maybe still a bit jank. I know I started it. Good enuf tho.

Make these promise chains work how I wanted a headache :(

DarkPurple141 · 2018-10-10T11:05:33Z

frontend/src/store/auth/index.js

    return new Promise((resolve, reject) => {
      const unsubscribe = auth.onAuthStateChanged(user => {
        unsubscribe()
        resolve(user)
      }, reject)
    })
-      .then(user => commit('SET_USER', user, { root: true }))
+      .then((user) => {
+        if (user === null) throw Error('Not logged in')


does it matter if user is set to null tho?

Oh for the below check.

DarkPurple141 · 2018-10-10T11:06:47Z

frontend/src/store/auth/index.js

      .catch(error => {
-        commit('ERROR', error)
+        commit('ERROR', error.message)


Offt this guy is pretty loaded.

he's a big boi

DarkPurple141 · 2018-10-10T11:07:21Z

frontend/src/store/auth/index.js

-        commit('ERROR', error)
+        commit('ERROR', error.message)
+        commit('SET_PROFILE', {}, { root: true })
+        if (!error.code || error.code !== 403) {


Can this ever occur?

403 is specifically the error we get when there's no profile, but the auth is valid otherwise.
I haven't actually thought about if checkAuth can trigger other errors from our backend, but meh.

frontend/src/store/root.js

DarkPurple141 · 2018-10-10T11:21:59Z

frontend/src/views/SignUp.vue

+    >
+      <AuthInput spellcheck="false" type="text" v-model="displayName" placeholder="Choose a display name"/>
+    </AppAuthForm>
+    <div v-else-if="!loading">


You really should do this not logged in thing in the router rather than the page. Vue-Router has a thing called navigation guards for this reason. Given we're already hiding the menu this seem like overkill potentially.

The problem is that logging in takes time and isn't always triggered by a button press. Whenever the app reloads, checkAuth happens and takes time to log in (refresh, typing in a URL etc).
Doing stuff on route doesn't work in this case... it needs to be reactive to the loading state.

backend/src/controllers/course.js

DarkPurple141

I think it's mostly fine, I'm just not sure on the global loading state (near sc) use and there's still a flash on intiial load between profile nav displaying and not.

DarkPurple141 · 2018-10-12T05:10:14Z

backend/src/controllers/course.js

@@ -59,7 +59,7 @@ exports.getCourseReviews = function ({ params, query }, res) {
                'meta': {


why are these quotes? seems like extra hassle.

not in pr :'( @Aruzal

DarkPurple141 · 2018-10-12T05:11:42Z

backend/src/controllers/review.js

@@ -68,7 +68,7 @@ exports.getReplyLikes = function ({ params }, res) {

 /* PUT updated reply likes value */
 exports.putReplyLikes = function ({ user, params, body, query }, res) {
-    body.userID = user || ANONYMOUS
+    body.userID = user && user.id || ANONYMOUS


Just looking at this.. Is there ever a reason you need more than userID? If not simple to put a middleware to expose this to all controllers.

Sidenote, are we maintaining support for anonymous? Also if we are, does anonymous have details associated with them.

this is temporary until we're authenticating all put/post/delete routes.
anonymous won't be supported after that imo

DarkPurple141 · 2018-10-12T05:13:31Z

backend/src/models/question.js

@@ -29,7 +29,7 @@ class Question {

    /**
     * Gets the total number of questions for a course
-     * @param   {string} code        The code of the course
+     * @param   {string} code        The code of the course duh


important change

it really was

DarkPurple141 · 2018-10-12T05:13:56Z

backend/src/models/user.js

     * @param {string} id The id of the auth'd user
     */
    getProfile(id) {
        return this.db
-            .query('SELECT * FROM user WHERE id=?', [id])
+            .query('SELECT id, email, displayName, degree, gradYear, description, picture, joined FROM user WHERE id=?', [id])


curious what picture is being stored as? Base64?

it's just a URL for now :)

kk may not ever be used XD

DarkPurple141 · 2018-10-12T05:37:36Z

frontend/src/store/root.js

-  isLoggedIn: ({ userAuthObject }) => !!userAuthObject,
-  authObject: ({ userAuthObject }) => userAuthObject
+  loading: ({ auth, course, questions, reviews, subject }) =>
+    [auth.loading, course.loading, questions.loading, reviews.loading, subject.loading].reduce((acc, curr) => acc || curr, false),


Not sure what I think of this. It's quite distracting seeing the spinner at top all the time, and means we don't lean on the substates.

i kinda put it in for testing.
visually it's not ideal but it is actually handy for debugging - maybe i should just comment it out of the nav bar?

could work yeah.

DarkPurple141 · 2018-10-12T05:38:25Z

frontend/src/views/ForgotPassword.vue

+            :buttonText="'Reset Password'"
+            :error="error"
+            >
+            <!--h3><i class="material-icons">warning</i> Warning </h3-->


Just remove the icon.

DarkPurple141 · 2018-10-12T05:39:42Z

frontend/src/views/Login.vue

@@ -1,6 +1,6 @@
 <template>
 <div class="auth-page">
-    <AppAuthForm v-if="!loading"
+    <AppAuthForm v-if="!loading && !isLoggedIn"


? I thought we were getting rid of this.

yes this is redundant. removed

DarkPurple141 · 2018-10-12T05:40:14Z

frontend/src/views/Profile.vue

+
+        <div class="profile">
+        <Card>
+            <div v-if="!loading && isLoggedIn">


What happens if not logged in?

this check is redundant. removed

DarkPurple141 · 2018-10-12T05:46:21Z

frontend/src/views/SignUp.vue

@@ -1,19 +1,30 @@
 <template>
  <div class="auth-page">
-    <AppAuthForm v-if="!loading"
+    <AppAuthForm v-if="!loading && !isFirebaseAuthorised"


hnnngg yeah okay i'll split it to an onboarding view i guess?

DarkPurple141

I think this looks fine.

@nun

* S6 db refactor (#188) * dealt with errors, dealt with redundant api calls * backend refactor start * Learning how2db * Less debug output * Autobackup db * backup kida working xD * Testing fixes - still not perfect but meh * Permanent dbs * Looks like we working * Update courses.js removed comment * Update stub-db.js removed old code * Fixed search bug whilst here * Fixed some of the issues * Debug info removed * Prod now not prod * S5 frontend pagination (#166) * Backend pagination added * backend pagination + basic error check * Fixed the check (swear it worked on mine lol) * Pagination for Questions/Reviews * Fixed lint issues * Backend pagination fixes * Moved some of the pagination logic out to a component * Backend didn't merge properly before * Lint fixes * Frontend merging with pagination * frontend pagemeta state * Backend dependent on pageSize * Tests had to be changes because response now has {meta, data} instead of just an array * linter yelling at me * Made page buttons sligtly more attractive * Fixes based on review * PageSelector only shows if there are multiple pages avail * Fixed typo's when copying * S7 Further Design Normalisation (#189) * further normalisation of designs * more normalising on fonts * Copy update * updates to home page * added robots.txt * Course Ratings Frontend (#193) * added an arc for ratings * getting my head around svgs * Ratings * now with animations * simplified reviews * S6 better reviews (#194) * Slidey boy * Added course recommend and fixed up slidey boy * Added reccomend to tests * backend post review stuff - untestederino * seemingly working review ratings added to db * fixed enjoy values in backend * working course ratings! (kinda) * rating circles stay up to date now * fixed review POST test * review option columns changerino - now fills whole width of review form * S7 upvotes (#209) * Basic PUT request working * Started frontend for things, not properly conencted, bit more work needed * Up and down voting working * Upvote reviews too * Questions, reviews, answers and replies all can be up/downvoted * Pass dem tests * Im a bad merger * Cleanup * Constants for Alex * S8 review card recommended basic (#207) * ugly thumbs to denote review recommended or not * smol fix on aggregation of recommended * fixed smol linting thing * slightly less bad thumb position * reverted changes to review card to keep this branch useful * hide ratings if no reviews yet * clear comment form after answering * fixed test * S8 Design Updates (#211) * ugly thumbs to denote review recommended or not * smol fix on aggregation of recommended * fixed smol linting thing * argghhh * slightly less bad thumb position * more progress * more designinggg * more fixes * okay moving on to the feeds now * slowly getting there * removed old firebase key * next step is to tackle the feed cards * added caching * reverted changes to review card to keep this branch useful * hide ratings if no reviews yet * clear comment form after answering * adjust feed * oops * S8 db dev data (#212) * Questions and Reviews * Comments populating * Done comments * Fixed tests to be more generic * Made test data longer and varied length * S8 Card Feeds (#219) * Questions and Review Feed Updates * changed smartcourse.azurewebsites.net to smartcourse.me (#226) * Shorter Course Information Data (#225) * S8 signup flow basic profile (#213) * signup now in two stages; create firebase account, and create profile * signup flow more logical now ... handles most edge cases pretty well * basic profile page - still buggy, non functional update profile button * local default picture * update user backend added... no tests * lint errors fixed * working profile update * fixed textarea stuff, watcher to fix refresh profile problem * profile picture update added * working password reset from login form * nice error message for non-unique username * made hasProfile dependent on isLoggedIn to alleviate some potential bad * removed TODO * fixed post review/question bug when logged in * loading state for nav, split auth and profile get into two bits * reverted some stuff ... working again * better auth code, profile cache, global loading spinner in nav, reroutes * lint * page size fixaroo * I'v messed something up * protecting routes with guards * fixed some PR things * smol fixes * removed preAuth route check - not needed rn * smol fix to login page reroute * Fixed feed lieks (#229) * S9 reduced descriptions (#230) * some simple filtering * Slightly better descriptions * S8 comment pagination (#208) * Backend pagination for comments * Cleaned up exiting controller pagination code and lint/test fixes * Oops, fixed that * Does this fix it * fixed test - body is now meta+data * Fixed breaking test * Revert "S8 comment pagination (#208)" (#235) This reverts commit 68bd7da. * S8-Design Tweaks Part 4 (#231) * Reworked Main Card Views * Built out new Components * More Design Refactoring * S8 restrict api (#232) * Simple auth safety * Working auth restrictions * Get an account * working progress * promt to login * other fixes * Damn tests * That ones on me * S9 loading state (#236) * loading state on all views, only get courses once on app load * course map + array lots of storage used... but should be speedier! * separated loading feed state from loading course info * pretty sure the beforeRouteUpdate re-load isn't needed on courses * split up question view loading into question and answers * split loading state in reviews * lint * kinda nice loading transitions in most places, plus removed a thing * performance * S9 time published (#237) * Date is in the form X <time> ago * Fixed UTC/LOCAL offsets * Fixed import * S9 Button Fixes (#239) * button fixes * Posts have user info (#238) * user Object added to all questions, reviews comments, added fake users * small improvement to courses efficiency * answers and replies now have user data, added reputation to user data * mappers for question and review get user object now.. * feed cards get username correctly now! * user data on all cards now * disabled vote buttons completely if not logged in * hide degree if user hasn't entered one * better dummy data * picture in Mini - but not actually hooked up cos cbf css * fixo testo! * lint * smol bugfix prob my bad * S9 SQL Optimisations (#243) * Optimisations to SQL Queries * S9 question card num answers (#241) * added numReplies to api query for questions feed * way too much effort adding names, degrees n junk hehe * question feed now shows number of answers! * course ratings update on GET * disabled like buttons when not logged in... * likes added to everything * minimum 0 for 'x users found this helpful' * removed profile caching, fixed post buttons bug, Create Profile nav * pr fixesszz * Pagination bug fix for off by 1 (#240) * Faculty degree data (#246) * Degree data * degrees and faculties in database and nice and routed af * bugfix on getting course info * S9 Comment Feed Rework (#247) * sort answers by hotness and put most upvoted at the top * removed toggle of classes & fixed a bug * fixed post answer test * answer section not hidden if 0 answers * bit of code cleanup, nicer loading state * moved answer loading state above answers - even better! * more cleanup, fixed disable messages, accepted answer border * replies now sorted by hotness (no accepted answer), reply form updated * course control button tooltip fix * S9 unlikes (#252) * Simple auth safety * Working auth restrictions * Get an account * Progressss * Working * remove some logs * forgot this log * working now it seems * Yeah don't know how that got back * very basic profiles without attempting to make anything pretty (#242) * very basic profiles without attempting to make anything pretty * removed a comment * Revert "very basic profiles without attempting to make anything pretty (#242)" (#256) This reverts commit e07dc11. * S9 faculty degree (#253) * Frontend store stuff * Caching * Caching actually works now :) * Removed loading as it should load once on init * S9 tos (#254) * Based func for ToS on Signup * A little bit of formatting * Privacy Policy & Tabs * S9 Review Overview (#258) * Amended likes logic * lots of updates -- recommend now showing, failing one test in user * S9 reputation (#255) * Simple auth safety * Working auth restrictions * Get an account * Progressss * Working * remove some logs * forgot this log * dem reps * working now it seems * Yeah don't know how that got back * for alex * im dum dum * bless the nunez * whoopsie * Works helllllz yeah * don't be negative * one bug to fix stilll * bug * bout time * I HATE JAVASCRIPT * pluralization correct for feed cards now (#259) * fixed bug with likes toggle functionality on refresh question/review (#261) * S9 course list filter search & sorting options (#265) * Footer linked up or cursor disabled (#264) * Footer linked up or cursor disabled * Fixes * Added, Fixed & Tested * S9 Public User Profiles (#257) * very basic profiles without attempting to make anything pretty * removed a comment * lint * the great Number String debacle of 2018 ... fixed for now. * progressing on profile * updates to login, signup, create-profile visuatiolns * fixed branch * minor feature addition * progress commit for @nun * somewhat improve myprofile page * small header fixup * nuno plz make better * griiiiid * slice that join! * S10 profile polish (#267) * more consistency updates * Category component bug (#268) * fixed the bug... * fixed properly now... * fix for page selector * Updates to Button Style for sorting (#277) * Bugfixes for demo data initialisation (#273) * dummy user data now has reputation and uses degree data * lint frontend * lint backend * fixed some promise bugarinos and added timers to initialization * dummy reviews now initialised with all ratings present * many speedup, due to bulk insert and limiting num courses populated * bugfix recommend text in review card * sort courses bugfix * fixed Recommendeds * sort fix for answers * Move DB init over to SQL (#276) * changes to db init * tests now pass * not quite there, added user init, but some issue with questions not being all inserted * hmmm * hmm * fixed * tightening tests again * minor updates * integrating nuno's development server changes * still a bit shit to be usable in development with the 30s lag * upgrade to version 0.1.0 * S10 sqlerise db init 2 (#279) * better * less quries * moooore * fix 1 trav error * fixed test rep * just seeing * just seeing 2 * just seeing 3 * Istanbul excludes more files for coverage and reports (#288) * excludes more files * amended cors, added basic tests * Added more tests, cors bugfix (#290) * Standardise Backend API to use constants, fix for time on Safari (#291) * initial commit * at least * fixed * need todo a larger refactor for likes * Better test data (#293) * initial commit * at least * fixed * need todo a larger refactor for likes * big changfe * Minor bug from dev init (#295) * init * fixed * forgot this

NunoDasNeves added 10 commits October 9, 2018 17:59

signup now in two stages; create firebase account, and create profile

afdd476

signup flow more logical now ... handles most edge cases pretty well

787eb20

basic profile page - still buggy, non functional update profile button

6c642d3

local default picture

cc0e33d

update user backend added... no tests

bfb209f

lint errors fixed

e5d0168

working profile update

4054dd3

fixed textarea stuff, watcher to fix refresh profile problem

bb48c8b

profile picture update added

daaa287

working password reset from login form

5293d8c

NunoDasNeves requested review from DarkPurple141, Aruzal and Papa-au October 10, 2018 06:48

NunoDasNeves added 2 commits October 10, 2018 18:05

nice error message for non-unique username

7bb0fd4

made hasProfile dependent on isLoggedIn to alleviate some potential bad

0d5b3bb

DarkPurple141 previously approved these changes Oct 10, 2018

View reviewed changes

removed TODO

6acbb76

NunoDasNeves dismissed DarkPurple141’s stale review via 6acbb76 October 10, 2018 23:58

NunoDasNeves added 3 commits October 11, 2018 11:42

Merge remote-tracking branch 'origin' into s8-signup-flow-basic-profile

e493132

fixed post review/question bug when logged in

2734f76

loading state for nav, split auth and profile get into two bits

dc2d329

Aruzal reviewed Oct 11, 2018

View reviewed changes

backend/src/controllers/course.js Outdated Show resolved Hide resolved

NunoDasNeves added 4 commits October 11, 2018 23:07

reverted some stuff ... working again

300d3a7

better auth code, profile cache, global loading spinner in nav, reroutes

08b0a35

lint

0ebe595

page size fixaroo

eab3230

DarkPurple141 requested changes Oct 12, 2018

View reviewed changes

Alex Hinds added 2 commits October 12, 2018 17:12

I'v messed something up

0621fe2

protecting routes with guards

2678b5a

NunoDasNeves added 3 commits October 12, 2018 17:31

fixed some PR things

1b0ae62

Merge branch 's8-nuno-helper-branch' into s8-signup-flow-basic-profile

160fc19

smol fixes

d628f8d

DarkPurple141 previously approved these changes Oct 13, 2018

View reviewed changes

removed preAuth route check - not needed rn

b9f9551

NunoDasNeves dismissed DarkPurple141’s stale review via b9f9551 October 13, 2018 05:25

NunoDasNeves added 2 commits October 13, 2018 16:26

Merge remote-tracking branch 'origin' into s8-signup-flow-basic-profile

d4c8a38

smol fix to login page reroute

b3f6134

NunoDasNeves merged commit d639762 into dev Oct 13, 2018

NunoDasNeves deleted the s8-signup-flow-basic-profile branch October 13, 2018 05:32

		@@ -59,7 +59,7 @@ exports.getCourseReviews = function ({ params, query }, res) {
		'meta': {

S8 signup flow basic profile #213

S8 signup flow basic profile #213

Conversation

NunoDasNeves commented Oct 10, 2018 • edited Loading

coveralls commented Oct 10, 2018 • edited Loading

Pull Request Test Coverage Report for Build 834

💛 - Coveralls

DarkPurple141 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NunoDasNeves Oct 10, 2018 • edited Loading

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

Choose a reason for hiding this comment

DarkPurple141 left a 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

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

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

DarkPurple141 left a comment

Choose a reason for hiding this comment

NunoDasNeves commented Oct 10, 2018 •

edited

Loading

coveralls commented Oct 10, 2018 •

edited

Loading

NunoDasNeves Oct 10, 2018 •

edited

Loading