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

added business requests #17

Merged
merged 4 commits into from
Jan 9, 2024
Merged

added business requests #17

merged 4 commits into from
Jan 9, 2024

Conversation

gdavid7
Copy link
Contributor

@gdavid7 gdavid7 commented Dec 6, 2023

No description provided.

@gdavid7 gdavid7 linked an issue Dec 6, 2023 that may be closed by this pull request
Copy link
Collaborator

@joshualipton joshualipton left a comment

Choose a reason for hiding this comment

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

Overall, really solid PR, this was a large task so great job handling that. Just some minor changes to be made, good work fellas, Christmas bonus incoming!!!

.pnp.cjs Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not be adding these files to the commits

.pnp.loader.mjs Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same with this file, we don't want to add this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same with this file

@@ -34,4 +34,239 @@ businessRouter.get('/:id', async (req, res) => {
}
});

// POST add a new business
businessRouter.post('/', async (req, res) => {
console.log(req.body);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: We shouldn't have console logs in our final commits

createdDate,
city,
} = req.body;
console.log(req.body);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Console log same as before

);
res.status(200).send(newBusiness);
} catch (err) {
res.status(400).send(err.message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should send a 500 error not a 400 error

// Sending a response
response.status(200).send('Update successful');
} catch (err) {
response.status(400).send(err.message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a 500 error


const newBusiness = await db.query(
`INSERT INTO business (
id, type, name, street, zipCode, state, qb_vendor_name, qb_city_state_zip,
Copy link
Member

Choose a reason for hiding this comment

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

Remove the id, we figured out how to auto increment it.

VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16,
$17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28, $29, $30,
$31, $32, $33, $34, $35, $36, $37, $38, $39, $40, $41, $42, $43, $44,
$45, $46, $47, $48, $49, $50, $51, $52, $53, $54, $55)
Copy link
Member

Choose a reason for hiding this comment

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

Just remember to remove $55 when you are removing the id

@Madhu2244 Madhu2244 merged commit 5898fff into dev Jan 9, 2024
2 checks passed
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.

setup_business_post_put_delete
3 participants