fullstackacademy / fsg Goto Github PK

https://github.com/FullstackAcademy/fsg/blob/master/generated/browser/js/fsa/fsa-pre-built.js#L93 returns a rejected promise with an object that has a message. Is there any compelling reason to do this over throw Error('message string')?

I have on a couple of occasions typed fsg intending to cd into a folder named fsg (my shell doesn't require the cd command) but accidentally activated the fsg command instead. This is pretty annoying as it then places all the generated files into my current folder, requiring careful cleanup.

Admittedly this may be a minority case as probably >50% of people either must type cd or choose to type cd. However, even for that majority of people, they may expect fsg to create a folder and accidentally place app files in a parent folder instead.

I propose the following behavior:

• fsg — makes a new app in a new fsg folder by default.
• fsg path/to/folder — makes a new app in path/to/folder (recursively constructing the path if it doesn't already exist).

It wouldn't be a difficult addition; if you approve the idea I'd be happy to make the PR.

Currently, the default task for Gulp includes setting a watcher to run testServerJS. However, the directories to watch doesn't include the server directory, only the tests/server directory, even though the comment on the line above specifies that it should run on any changes to either the server or tests/server directory.

Static pixel width causing:

res.send({ user: _.omit(req.user.toJSON(), ['salt', 'password']) });

this omission functionality should probably be centralized as a method.

Don't use this anymore.

Pursuant to a conversation between @joedotjs and myself, the no-debugger rule — currently set to level 2 (error) to prevent committing a debugger statement — causes the lint task to fail, in turn stopping the build.

A solution would be to separate dev linting from production linting. The former could have no-debugger set to level 1 (warning), letting students know it should be removed prior to commit; the latter could have no-debugger set to 2 (error), providing an early out for build.

To accomplish this, gulp-eslint can take an options object using the same CLI options as specified here. The configFile option will let us programmatically select which eslintrc to apply. Alternatively — and perhaps preferably? — the rules option can let us add production-specific rules to override the base config.

There may also be other alternatives; research accordingly.

E.g. with istanbul.

It would appear something is wrong with sessions.

  AuthService
    ✔ should be an object
    isAuthenicated
      ✖ should return true if a Session exists
      ✔ should return false if a Session does not exist
    getLoggedInUser
      ✖ should return the user from the Session if already authenticated
      when user not already authenticated
        ✔ should make a request to GET /session
        ✔ should resolve to the reponse from /session if it is OK
        ✔ should resolve to null if the response was 401/erroneous
        on successful response
          ✔ should resolve to the responded user from /session
          ✖ should set the session
        ✖ "after each" hook for "should set the session"
    login
      ✔ should make a request POST /login with the given data
      ✔ should forward invalid credentials error on unsuccessful response
      on successful response
        ✔ should resolve to the responded user
        ✖ should set Session
      ✖ "after each" hook for "should set Session"
    logout
      ✔ should make a request GET /logout
      ✔ should destroy the session
      ✔ should broadcast the logoutSuccess AUTH_EVENT
  Session Service
    ✔ should be an object
    ✖ should by default have id and user as null
    create method
      ✖ should when called with id and user argumentsset the id and user to session
    destroy method
      ✖ should set user and id to null
    event listening
      ✔ should call destroy when notAuthenticated event is fired
      ✔ should call destroy when sessionTimeout event is fired

1604's Jason Hang discovered this bug which causes the user model to get redefined without any fields when facebook oauth is turned on. Pretty crazy, total ninja bug!

Facebook uses a clientId in the strategy, but an APP_ID in the production environment variable. See https://github.com/FullstackAcademy/fsg/blob/master/generated/server/env/production.js#L18. While students should notice the difference and put the right vars in Heroku, it is pretty easy to overlook this as it is similar but not identical to the Google variable names.

Not sure why it is coded in as APP_ID — is it to match some Facebook dev tool verbiage? An oversight? Worth looking into to see if it wouldn't be less confusing standardized to CLIENT_ID.

Eventually the standard Node version will contain enough ES6 so that this becomes moot, but at least for now, FSG transpiles server & browser code but not test code. This can cause problems e.g. on Codeship if people are using the (current) default Node env of v5. A few possible solutions, one of which is already in place:

• Make sure the Codeship testing wiki instructions include nvm use v6 or similar. This is currently in effect, but is annoying to hard-code into the wiki — by rights this line probably shouldn't exist.
• Make sure the generated package.json lists a node version in engines that is ES6 compatible, as Codeship reads that file when starting a build. If we transpile tests, this isn't necessary.
• Transpile tests. This maintains parity with server & browser code and lets us easily update the transpiler for the entire project in the future, e.g. ES7 -> ES6 or similar. This is probably the best route though it is not mutually exclusive with the other options.

Since the custom logging middleware exists from David, morgan should be removed as a dependency.

Including neutralizing the "timing" aspect of instantiation and providing an example of collecting/sharing connected sockets and matching those sockets with authenticated users.

It's a known bug/feature/who knows that the beforeValidate method in sequelize runs twice.

I ran into this when trying to allow users to change passwords. When creating a user the if(user.changed('password')) returns true the first time beforeValidate runs - this works fine. It does run again, but user.changed('password') returns false so we don't get into the if block again.

However when actually "updating" a user's password, the beforeValidate runs twice and hits that if statement twice. This causes the users's plain text password to be hashed once, then the beforeValidate hook runs the second time and the user.changed('password') returns true again. Now the user's password is set to a hash of the hash of their original plaintext password.

The only way for the user to login after this is to obtain the first hash of their plaintext password and input that into the password field.

beforeUpdate only runs once and seems to be the better solution. I propose a change in the user model hooks to something like the below:

    hooks: {
        beforeCreate: function (user) {
            user.salt = user.Model.generateSalt();
            user.password = user.Model.encryptPassword(user.password, user.salt);
        },
        beforeUpdate: function(user){
          if (user.changed('password')) {
              user.salt = user.Model.generateSalt();
              user.password = user.Model.encryptPassword(user.password, user.salt);
          }
        }
    }

This was my workaround, but if someone knows a better way please let me know.

FSG by default loads the home state upon initialization. This makes it difficult to unit test components. To have working front-end tests, students must either:

• Architect all their services as belonging to a standalone Angular module, à la fsgPrebuilt.
• Use a preprocessor and accept that the home state + all its dependencies are going to be bootstrapped (bad idea)
• Add a testing harness that detects the node environment, sets some window flags, and configures the app accordingly (probably the easiest).

This is, of course, specific to Angular and we are probably going to abandon Angular-FSG for React-FSG. However if we continue to use the Angular version of this app, we should probably implement the third option above to make it easier on students.

new version of ui bootstrap available in a different npm module. Supports popover templates and other features
angular-ui-bootstrap(https://www.npmjs.com/package/angular-ui-bootstrap)
instead of the current angular-bootstrap (https://www.npmjs.com/package/angular-bootstrap).
also requires ng- animate https://www.npmjs.com/package/angular-animate

Session.create is being called without a sessionId on line 55 in fsa-pre-built.js. This results in continuous get requests to /session after user has logged in since this.isAuthenticated() returns false on Line 76.

Had a problem where the version of Gulp/Node Sass is breaking when running Node 4.0.0. I deleted in package.son gulp-sass then ran npm install gulp-sass --save-dev and then reran nom install. I believe the version number needs to be adjusted to the latest release of gulp-sass. This also works on computers still running Node 0.14.

The spec files connect to a testing database. They also pull in the express app by passing that database object, which pulls in the api router which pulls in the sub-route handlers. For example people will generally build a users router and plug it in here. But if so, then that router will need the user model to work, and so people will require in db/index.js. Which will trigger another database connection. So the express app at the top level will be talking to the testing database, but its api route handlers will end up talking to the dev database.

I think if we want to be able to define the database object from the spec file, we'd need the express app to pass it down to the routers as well. OR—and this gets my vote—we could have a testing.js file inside of server/env and then server/env/index.js would also have to take that into account when deriving its exports.

On page: https://github.com/FullstackAcademy/fsg
wiki link: https://github.com/FullstackAcademy/fsg/wiki/01.-Getting-Started
should be: https://github.com/FullstackAcademy/fsg/wiki/Getting-Started

My npm was having trouble installing angular-bootstrap, and it seems the actual module is called "angular-bootstrap-npm". I changed my own package-json and it fixed the problem.

Students very often make the mistake of putting things in public, or changing files in public — often to the point of removing public from .gitignore. Part of the problem is we've used public for static routing during the curriculum, but then we switch it up and use public as our build folder for FSG.

We should change the name of this folder to something less tempting — e.g. build. It won't prevent students from messing with it, but it will probably reduce the likelihood at least.

Doing the FSG auth session right now and just noticed that there's an exec method call on the twitter oauth config file on line 30. Not on google, probably a mongoose relic?

So in general with sequelize you need a static reference to the database object to define models and you need a static reference to various models to define relevant associations. This sort of means you can't define the associations in the same file that you define the database.

There are two common solutions we've employed in our codebases: 1) make the model files export functions that take the database object and define the model; 2) make a _db.js file (or equivalent) that defines the database object internally, and a "models index file" that defines the associations and also exports the db for ease of use by external components.

Right now in the generated files we're doing both (@andrewmash referred to this as the "belt and suspenders" approach which made me lol). Am I missing something or can we just instead do one of these? If so, which one (my vote is for number 2)?