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

fix: add sector value to sector insight api #76

Merged
merged 7 commits into from
Feb 29, 2024
Merged

fix: add sector value to sector insight api #76

merged 7 commits into from
Feb 29, 2024

Conversation

chominho96
Copy link
Member

Issue Number

close: #75

작업 내역

구현 내용 및 작업 했던 내역

  • sector insight api에 sector value를 넣을 수 있도록 수정

변경사항

  • 의존성 목록

PR 특이 사항

PR을 볼 때 주의깊게 봐야하거나 말하고 싶은 점

  • 종목_상세_정보의_배당날짜를_올해기준으로_반환한다() 테스트가 저번에 있었던 날짜 이슈와 비슷한 이유로 fail이 나더라구요!! 그래서 수정했습니다!!

@chominho96 chominho96 added the bug Something isn't working label Feb 29, 2024
@chominho96 chominho96 requested a review from songyi00 February 29, 2024 08:34
@chominho96 chominho96 self-assigned this Feb 29, 2024
Copy link
Member

@songyi00 songyi00 left a comment

Choose a reason for hiding this comment

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

코멘트 남겨놓았습니다 👍

.groupBy(stock.id, stock.price)
.orderBy(dividendYield.desc())
.having(dividendYield.loe(MAX_DIVIDEND_YIELD))
Copy link
Member

Choose a reason for hiding this comment

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

혹시 loe가 low or equal 이 맞다면 이하가 아니라 미만이 되어야 할 것 같습니다!! :>

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 수정했습니다!!

CONGLOMERATES("Conglomerates"),
ETF("ETF"),
ETC("ETC");
TECHNOLOGY("Technology", "TECHNOLOGY"),
Copy link
Member

Choose a reason for hiding this comment

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

요거 value 추가된 이유가 있을까요?? enum 과 동일해서 그대로 사용해도 될 것 같아서요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 혹시 저 enum 값을 그대로 사용할 수 있는 방법이 있나요?! String으로 받아서 비교하려면 저는 필요한걸로 생각했어요...!!

Copy link
Member

Choose a reason for hiding this comment

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

넵 enum 그대로 반환하면 될 것 같아용~!

Copy link
Member Author

Choose a reason for hiding this comment

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

넵넵!!

int expectedMonth = 3;
int expectedDayOfMonth = 1;
int lastYear = LocalDate.now().getYear() - 1;
Instant exDividendDate = LocalDate.of(lastYear, 3, 1).atStartOfDay().toInstant(UTC);
Copy link
Member

Choose a reason for hiding this comment

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

수정 감사합니다 🙇🏻‍♀️

Copy link
Member

@songyi00 songyi00 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다👍👍
enum만 수정 부탁드려욧

@chominho96 chominho96 merged commit 3847905 into develop Feb 29, 2024
1 check passed
@chominho96 chominho96 deleted the feat/#75 branch February 29, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] sector insight api에 sector 정보 추가
2 participants