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

SeedDB script is working now #93

Merged
merged 8 commits into from
Jan 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 27 additions & 22 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
{
"parserOptions": {
"ecmaVersion": 6,
"ecmaVersion": 2017,
"sourceType": "module",
"ecmaFeatures": {
"jsx": true
}
},
"plugins": [
"react"
],
"plugins": ["react"],
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious but why did you change this to inline?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only one item, can fit in one line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a prettier preset. I think 80 characters is the limit for making line breaks. All the formatting changes were auto-saved by prettier so there is a lot of noise :(

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this prettier preset remove use of arrow functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it doesn't, but it defaults to a style of param => { ... } instead of (param) => { ... }. The methods on the models use a traditional function because I believe those methods will need this to point to the model in order to do stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For multiple params, it will always be in (param1, param2) => {} because javascript requires that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, i saw some arrow functions on the other files. i saw one change from () => {} to function() {}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In most cases, we can use arrow functions but there are a few edge cases where we need a function declaration in order to access this correctly or accessing arguments within the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still figuring those edge cases out. I remember some guys running into issues at work with 'this' binding in es5.

"extends": "plugin:react/recommended",
"rules": {
"camelcase": "error",
Expand All @@ -29,13 +27,17 @@
"comma-spacing": ["error", { "before": false, "after": true }],
"comma-style": ["error", "last"],
"eqeqeq": ["error", "smart"],
"indent": ["error", 2, {
"VariableDeclarator": {
"var": 2,
"let": 2,
"const": 3
"indent": [
"error",
2,
{
"VariableDeclarator": {
"var": 2,
"let": 2,
"const": 3
}
}
}],
],
"dot-notation": "error",
"global-require": "error",
"no-cond-assign": "error",
Expand All @@ -60,19 +62,22 @@
"one-var-declaration-per-line": ["error", "initializations"],
"quotes": ["warn", "single"],
"semi": ["error", "always"],
"spaced-comment": ["error", "always", {
"line": {
"markers": ["/"],
"exceptions": ["-", "+", "*"]
},
"block": {
"markers": ["!"],
"exceptions": ["*"],
"balanced": false
"spaced-comment": [
"error",
"always",
{
"line": {
"markers": ["/"],
"exceptions": ["-", "+", "*"]
},
"block": {
"markers": ["!"],
"exceptions": ["*"],
"balanced": false
}
}
}],
],
"use-isnan": "error",
"valid-typeof": "error"

}
}
}
7 changes: 3 additions & 4 deletions server/models/family.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,20 @@ const familySchema = new Schema({
wishlist: {
type: Schema.Types.ObjectId,
ref: 'Wishlist',
required: true,
},
size: Number,
});

familySchema.methods.createWishlist = function () {
familySchema.methods.createWishlist = function() {
const wishlist = new Wishlist();

wishlist.save(function(err) {
if (err) console.log(err);
});
};

familySchema.methods.readWishlist = function () {
return Wishlist.findOne({ 'Family': this._id }, function (err, wishlist) {
familySchema.methods.readWishlist = function() {
return Wishlist.findOne({ Family: this._id }, function(err, wishlist) {
if (err) {
console.log(err);
} else {
Expand Down
14 changes: 7 additions & 7 deletions server/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,33 @@ const userSchema = new Schema({
username: {
type: String,
required: true,
index: { unique: true }
index: { unique: true },
},
name: {
type: Schema.Types.ObjectId,
ref: 'Name',
required: true
required: true,
},
email: { type: String, required: true },
password: { type: String, required: true },
phone: String,
// Check below fields, if present, user is of that type
_donor: {
type: Schema.Types.ObjectId,
ref: 'Donor'
ref: 'Donor',
},
_family: {
type: Schema.Types.ObjectId,
ref: 'Family'
ref: 'Family',
},
_organizer: {
type: Schema.Types.ObjectId,
ref: 'Organizer'
}
ref: 'Organizer',
},
});

// Hash password before saving
userSchema.pre('save', next => {
userSchema.pre('save', function(next) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we losing the arrow functions? shouldn't this be (next) =>

Copy link
Contributor

Choose a reason for hiding this comment

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

As per the official mongoose docs:

Do not declare methods using ES6 arrow functions (=>). Arrow functions explicitly prevent binding this, so your method will not have access to the document and the above examples will not work.
mongoose docs: methods

bcrypt.hash(this.password, saltRounds, (err, hash) => {
if (err) return next(err);
this.password = hash;
Expand Down
12 changes: 6 additions & 6 deletions server/models/wishlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const Schema = mongoose.Schema;

const wishlistSchema = new Schema({
family: {
type: Schema.objectId,
ref: 'Family'
type: Schema.ObjectId,
ref: 'Family',
},
list: [
// An array of objects
Expand All @@ -15,10 +15,10 @@ const wishlistSchema = new Schema({
totalListCost: Number,
dateLastUpdated: {
type: Date,
default: Date.now
}
}
]
default: Date.now,
},
},
],
});

const Wishlist = mongoose.model('Wishlist', wishlistSchema);
Expand Down
Loading