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

[50기 홍영기 - ADD : 각 항목별 이달의 지출 조회 기능] #2

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

hoifoi
Copy link
Contributor

@hoifoi hoifoi commented Nov 13, 2023

1. 본 PR이 우리 팀의 웹 서비스 제품성에 어떠한 기여를 하였고,
사용자에게 어떠한 기대효과를 전달하는지 작성해주세요.

  • 내 PR이 제품 내 어떠한 기능적인 배경/전후맥락 가운데 개발되었나요?

로그인 후 처음 진입하는 페이지에서 가족 및 개인의 지출과 수입등의 변동의 표시를 위해 개발

  • 내 PR이 Merge 됨으로써 유저에게 전달되는 편익/기대효과는 무엇일까요?

로그인 후 처음 진입하는 페이지에서 가족 및 개인의 지출과 수입등의 변동의 표시를 위해 개발


2. 이 브랜치에서 어떤 내용을 개발했는지 큰 제목과 상세 내역을 적어주세요.

이번 달의 수입/지출 표시(가족)

이번 달의 수입/지출 표시(개인)

이번 달의 수입/지출 카테고리별로 표시(가족)

이번 달의 수입/지출 카테고리별로 표시(개인)


3. 개발한 화면을 캡쳐해서 첨부 해 주세요.


스크린샷 2023-11-13 오후 7 41 56

4. 이 브랜치에서 개발하면서 느꼇던 개발 성장포인트를 적어주세요.

복잡한 쿼리문의 작성에 대한 경험을 통해 mysql에 대한 부담을 덜게 되었다
단순 데이토 조회만으로는 프론트에서 표시하기엔 어려움이 있어 재가공의 필요성과 그 경험을 얻었다

@hoifoi hoifoi requested a review from soheon-lee November 13, 2023 10:43
@hoifoi
Copy link
Contributor Author

hoifoi commented Nov 13, 2023

@soheon-lee
알려주신 리뷰 사항 최대한 기억나는대로 반영하였습니다 :D


const settingRouter = require('./settingRouter')

router.use('/setting', settingRouter.router)

Choose a reason for hiding this comment

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

Suggested change
router.use('/setting', settingRouter.router)
router.use('/flows', moneyFlowRouter.router)

Comment on lines 6 to 7
router.get('/month/family/', settingController.viewMonthFamily );
router.get('/month/private/', settingController.viewMonthPrivate );

Choose a reason for hiding this comment

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

Suggested change
router.get('/month/family/', settingController.viewMonthFamily );
router.get('/month/private/', settingController.viewMonthPrivate );
router.get('/statistics', settingController.getMoneyFlowStatistics );

Comment on lines 3 to 13
const viewMonthFamily = async(req, res) => {
try{
const year = new Date().getFullYear()
const familyId = 1
const result = await settingServices.viewMonthFamily( familyId, year )
res.status(200).json( result )
}catch(err){
console.log(err)
res.status(err.statusCode || 500).json({message : err.message})
}
}

Choose a reason for hiding this comment

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

Suggested change
const viewMonthFamily = async(req, res) => {
try{
const year = new Date().getFullYear()
const familyId = 1
const result = await settingServices.viewMonthFamily( familyId, year )
res.status(200).json( result )
}catch(err){
console.log(err)
res.status(err.statusCode || 500).json({message : err.message})
}
}
const getMoneyFlowStatistics = async(req, res) => {
try {
const type = req.query.type
const period = req.query.period
const year = new Date().getFullYear()
const familyId = 1
const result = await settingServices.getMoneyFlowStatistics( type, period, userId, familyId)
res.status(200).json( result )
}catch(err){
console.log(err)
res.status(err.statusCode || 500).json({message : err.message})
}
}

const flowServices = require('../services/flowService')

const search = async(req,res) => {
try{

Choose a reason for hiding this comment

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

Suggested change
try{
try {

띄어쓰기 전부 적용해 주세요

Comment on lines +24 to +29
const {
year : year,
month : month = '',
rule : rule,
unit : unit
} = req.query

Choose a reason for hiding this comment

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

Suggested change
const {
year : year,
month : month = '',
rule : rule,
unit : unit
} = req.query
const {
year,
month = '',
rule,
unit
} = req.query

key와 value의 이름이 같을 경우 곧바로 구조분해 할당 하셔도 됩니다

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

Successfully merging this pull request may close these issues.

2 participants