Skip to content

Jesse Reichel — Sprint-Challenge-Mongo#258

Open
frankfaustino wants to merge 3 commits intobloominstituteoftechnology:masterfrom
CSPT1-TEAMS:jesse-reichel
Open

Jesse Reichel — Sprint-Challenge-Mongo#258
frankfaustino wants to merge 3 commits intobloominstituteoftechnology:masterfrom
CSPT1-TEAMS:jesse-reichel

Conversation

@frankfaustino
Copy link

No description provided.

@@ -0,0 +1,15 @@
1. Describe the following: `DataBase`, `Collection` , `Document`
DataBase: Is a large assortment of Collections and Documents; it represents the largest group in the mongo file structure.
Copy link
Author

Choose a reason for hiding this comment

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

👌

const router = express.Router();

router.post('/', (req, res) => {
const {title, budgetAmount} = req.body
Copy link
Author

Choose a reason for hiding this comment

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

Since we're not using the variables title and budgetAmount for anything, we could just pass req.body to our new Budget: const budget = new Budget(req.body) and skip the whole object destructuring thing.

})

router.post('/', (req, res) => {
const {title } = req.body
Copy link
Author

@frankfaustino frankfaustino Jun 15, 2018

Choose a reason for hiding this comment

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

Character spacing here is a bit off. According to Airbnb's styleguide (https://github.com/airbnb/javascript#whitespace--in-braces), there should be spaces inside curly braces.

Also, we don't need to use object destructuring here.

const Expense = new mongoose.Schema({
amount: Number,
description: String,
budget:[{
Copy link
Author

Choose a reason for hiding this comment

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

Since Expenses are many-to-one relationship with Budgets, this budget property shouldn't be an array.

type: ObjectId,
ref: "Budget"
}],
category:[{
Copy link
Author

@frankfaustino frankfaustino Jun 15, 2018

Choose a reason for hiding this comment

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

Same here, there are many Expenses per Category, but only one Category for each, so this property shouldn't be in an array.

})
})

router.post('/', (req, res) => {
Copy link
Author

@frankfaustino frankfaustino Jun 15, 2018

Choose a reason for hiding this comment

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

Instead of sending the budget and category ObjectId along with the POST request, we can send a POST request that looks like this:

{
	"amount": 35,
	"description": "potatoes",
	"budget": "Monthly Spending",
	"category": "Groceries"
}

In order to do that, we need to add some logic to our route:

.post((req, res) => {
  let { amount, budget, category, description } = req.body

  budget = Budget.findOne({ title: budget })
  category = Category.findOne({ title: category })

  Promise.all([budget, category])
    .then(([budget_id, category_id]) => {
      const expense = new Expense({
        amount,
        budget: budget_id,
        category: category_id,
        description
      })

      expense.save()
        .then(response => res.status(201).json(response))
        .catch(err => res.status(500).json(err)))
    })
    .catch(err => res.status(500).json(err))
  })

We have to get budget and category with the findOne method, then using Promise.all, we grab the ObjectId for each from the response from MongoDB, then create our new Expense.

@frankfaustino
Copy link
Author

frankfaustino commented Jun 15, 2018

Well done, Jesse! You've clearly got a good grasp of the material from last week.

I know it wasn't clear in the assignment's instructions that you have to fetch the budget and category ObjectIds before saving them to an Expense. But consider how the client (Postman, React app, or whatever) would even know what those ObjectIds are to begin with.

Otherwise, good job! Keep up the excellent work 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants