-
Notifications
You must be signed in to change notification settings - Fork 2
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
refactor the code phase1 NEEDS DISCUSSIONS #106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good piece.
Left some comments.
src/controllers/node.controller.ts
Outdated
count = searchResult.count; | ||
} | ||
|
||
res.status(201).json({ nodes, count }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the right status code here should be 200
as 201
is for creating new resource.
Can we update that pls.
res.status(201).json({ nodes, count }); | |
res.status(200).json({ nodes, count }); |
src/dao/api.dao.ts
Outdated
console.log("====imgs===", images); | ||
console.log(name); | ||
console.log(userId); | ||
console.log("===rest===", rest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets do away with the logs if not needful.
console.log("===rest===", rest); | ||
const createdClaim = await prisma.claim.create({ | ||
data: { | ||
issuerId: `http://trustclaims.whatscookin.us/users/${userId}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommending we store the baseURL (https://trustclaims.whatscookin.us) used in the issuerId here in an environment variable. This would enhance maintainability by enabling us to modify the URL centrally in case of future changes (e.g., domain migration, API updates) etc...
edgesTo: { | ||
some: { | ||
claim: { | ||
issuerId: `http://trustclaims.whatscookin.us/users/${userId}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
No description provided.