Set up send email #9
Conversation
kkinggmann
force-pushed
the
set_up_send_email
branch
6 times, most recently
from
c3b7caa
to
f91a815
Compare
|
@sagara11 @SalamanderHP pls review my pr |
|
@sagara11 @SalamanderHP pls check the pr |
client/.gitignore
Outdated
| @@ -24,3 +24,4 @@ npm-debug.log* | |||
| yarn-debug.log* | |||
| yarn-error.log* | |||
| src/contracts/ | |||
There was a problem hiding this comment.
@kkinggmann cái yarn này t bỏ ở pr t rồi mà, check lại xem bị thừa ko
server/models/users.js
Outdated
| @@ -46,4 +49,22 @@ User.statics.findByAddress = async function (publicAddress) { | |||
| return user; | |||
| }; | |||
|
|
|||
| User.pre("findOneAndUpdate", async function (next) { | |||
There was a problem hiding this comment.
@kkinggmann hàm findOneAndUpdate là hàm có sẵn rồi giờ m lại định nghĩa 1 hàm riêng của model làm gì, trong service có thể dùng nhiều hàm mặc định của model tùy úy chứ t nghĩ ko nên định nghĩa lại quá nhiều mấy hàm đơn giản vào model này
There was a problem hiding this comment.
@SalamanderHP k phải, cái này là callback thôi. Là trước khi findOneAndUpdate thì nó chạy. Giống mấy cái callback trong rails ý
There was a problem hiding this comment.
@kkinggmann ừ ko để ý cái pre, chỗ logic kia đưa vào 1 service nhé đừng viết kiểu next trong model
There was a problem hiding this comment.
service cũng không nên đưa next vào, luồng request thì ở controller hết nhé
There was a problem hiding this comment.
@SalamanderHP cái trong callback thì nếu dùng là cần viết next() đấy, để nó chạy được tiếp khi qua callback. Cơ mà chắc chỗ này không cần callback đâu
server/models/users.js
Outdated
| // Check only send email after fill second step form | ||
| if (isEmpty(preUpdatedUser.email) && !isEmpty(this.getUpdate().$set.email)) { | ||
| const email = this.getUpdate().$set.email; | ||
| sendEmailJob(email, `New email: ${email}`, "New account"); |
There was a problem hiding this comment.
@kkinggmann model thì không có logic nhé, tất cả những chỗ logic này nên ở service chứ không ở model
There was a problem hiding this comment.
@SalamanderHP oke để nghĩ lại chỗ này đưa vào service
server/services/UserServices.js
Outdated
| @@ -14,6 +14,17 @@ const addNewUser = async (publicAddress) => { | |||
| return user; | |||
| }; | |||
|
|
|||
| const updateUser = async (publicAddress, data) => { | |||
There was a problem hiding this comment.
@kkinggmann nên viết 1 đối tượng const userService rồi bên trong nó là các function, ở controller sẽ dễ đọc hơn
server/services/UserServices.js
Outdated
| addNewUser, | ||
| updateUser, |
There was a problem hiding this comment.
@kkinggmann viết thế kia thì chỉ việc export cái userService đó là bên controller sẽ implement như sau: userService.updateUser -> debug sẽ dễ tìm được hàm này thuộc service nào
| @@ -18,5 +18,5 @@ export function signupAPI(params) { | |||
| } | |||
|
|
|||
| export function updateAccountAPI(params) { | |||
| return axios.put(`${URL}/users/update`, params, config); | |||
| return axios.put(`${URL}/users/update/${params.publicAddress}`, params.data, config); | |||
There was a problem hiding this comment.
@kkinggmann cần gì phải có chữ update khi đã sử dụng phương thức put nhỉ
kkinggmann
force-pushed
the
set_up_send_email
branch
3 times, most recently
from
ea5cdf8
to
49e26ca
Compare
|
@SalamanderHP @sagara11 pls review |
| const {publicAddress} = req.params; | ||
| const data = req.body; |
There was a problem hiding this comment.
@kkinggmann trong phạm vi hàm để truyền vào update thì dùng let cũng được ko cần const, mà để thế cũng đc k cần fix
| const {publicAddress} = req.params; | ||
| const data = req.body; | ||
|
|
||
| const updatedUser = await UserServices.updateUser(publicAddress, data); |
There was a problem hiding this comment.
@kkinggmann service này nên để camelCase thôi object mà, thống nhất về sau nữa, fix cái này thôi còn đâu code quá clean rồi
kkinggmann
force-pushed
the
set_up_send_email
branch
from
49e26ca
to
d240562
Compare
No description provided.