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.

Sign up for GitHub

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

Analytics duplication refactor #4920

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Analytics duplication refactor #4920

Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
385d662
WIP: Add yearly ts wrapper.
jaredb96 Oct 22, 2024
c883956
WIP: Add quarterly ts wrapper.
jaredb96 Oct 22, 2024
07eb747
WIP: Replace delete ts row with wrapper.
jaredb96 Oct 22, 2024
7cb303e
WIP: Replace insert ts row with wrapper.
jaredb96 Oct 28, 2024
3fcad82
Add Lar repo wrapper and delete unneeded values.
jaredb96 Oct 28, 2024
e3a3ea4
Fix 2019 typo.
jaredb96 Oct 28, 2024
1f851b7
Fix 2019 typo.
jaredb96 Oct 28, 2024
85b9932
Remove unneeded params.
jaredb96 Oct 29, 2024
c41c260
Fix params.
jaredb96 Oct 29, 2024
64fd69c
Remove unneeded function.
jaredb96 Oct 30, 2024
fa870aa
Remove old quarterly lei function.
jaredb96 Oct 30, 2024
2f59941
Simplify cases in deleteTsRow.
jaredb96 Nov 1, 2024
eba19f0
Simplify cases in insertTsRow.
jaredb96 Nov 1, 2024
edb6c1e
Simplify cases in deleteLarRow.
jaredb96 Nov 1, 2024
4dd0be3
Simplify cases in insertLarRows..
jaredb96 Nov 1, 2024
31eca7a
Fix compilation error.
jaredb96 Nov 1, 2024
0b8d50b
Resolve merge conflict.
jaredb96 Nov 29, 2024
db184bb
Add 2025 Q1 for devtesting.
jaredb96 Nov 30, 2024
efa74b0
Revert 2025.
jaredb96 Nov 30, 2024
de39641
Revert 2025.
jaredb96 Dec 2, 2024
b267632
Merge branch 'master' into analytics-duplication-refactor
jaredb96 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 6 additions & 79 deletions hmda-analytics/src/main/scala/hmda/analytics/HmdaAnalyticsApp.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,85 +317,12 @@ object HmdaAnalyticsApp extends App with TransmittalSheetComponent with LarCompo
for {
insertorupdate <- submissionId.period match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we replace this very long match statement with a map? And maybe do the same for the other match statements here? It would cut down on a ton of lines of code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm could you clarify a bit what you were thinking?

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, map isn't the right thing here. Not sure what I was thinking yesterday.

What I meant is we can simplify this to 2-3 cases: case Period(year, None) => and case Period(year, Some(quarter)) => (and keep the final case _ => case). Using a variable for the year and quarter will get rid of a ton of these match cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah let's do that, will take some time to update

case Period(2018, None) => YearlyLarRepositoryWrapper("2018").getLarRepository.insert(LarConverter2018(lar))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

2018 has a special lar converter so we're keeping this here

case Period(2019, None) =>
YearlyLarRepositoryWrapper("2019").getLarRepository.insert(
LarConverter(lar, 2019)
)
case Period(2020, Some("Q1")) =>
QuarterlyLarRepositoryWrapper("2020", "Q1").getLarRepository.insert(
LarConverter(lar = lar, 2020, isQuarterly = true)
)
case Period(2020, Some("Q2")) =>
QuarterlyLarRepositoryWrapper("2020", "Q2").getLarRepository.insert(
LarConverter(lar = lar, 2020, isQuarterly = true)
)
case Period(2020, Some("Q3")) =>
QuarterlyLarRepositoryWrapper("2020", "Q3").getLarRepository.insert(
LarConverter(lar = lar, 2020, isQuarterly = true)
)
case Period(2020, None) =>
YearlyLarRepositoryWrapper("2020").getLarRepository.insert(
LarConverter(lar = lar, 2020)
)
case Period(2021, None) =>
YearlyLarRepositoryWrapper("2021").getLarRepository.insert(
LarConverter(lar = lar, 2021)
)
case Period(2022, None) =>
YearlyLarRepositoryWrapper("2022").getLarRepository.insert(
LarConverter(lar = lar, 2022)
)
case Period(2023, None) =>
YearlyLarRepositoryWrapper("2023").getLarRepository.insert(
LarConverter(lar = lar, 2023)
)
case Period(2021, Some("Q1")) =>
QuarterlyLarRepositoryWrapper("2021", "Q1").getLarRepository.insert(
LarConverter(lar = lar, 2021, isQuarterly = true)
)
case Period(2021, Some("Q2")) =>
QuarterlyLarRepositoryWrapper("2021", "Q2").getLarRepository.insert(
LarConverter(lar = lar, 2021, isQuarterly = true)
)
case Period(2021, Some("Q3")) =>
QuarterlyLarRepositoryWrapper("2021", "Q3").getLarRepository.insert(
LarConverter(lar = lar, 2021, isQuarterly = true)
)
case Period(2022, Some("Q1")) =>
QuarterlyLarRepositoryWrapper("2022", "Q1").getLarRepository.insert(
LarConverter(lar = lar, 2022, isQuarterly = true)
)
case Period(2022, Some("Q2")) =>
QuarterlyLarRepositoryWrapper("2022", "Q2").getLarRepository.insert(
LarConverter(lar = lar, 2022, isQuarterly = true)
)
case Period(2022, Some("Q3")) =>
QuarterlyLarRepositoryWrapper("2022", "Q3").getLarRepository.insert(
LarConverter(lar = lar, 2022, isQuarterly = true)
)
case Period(2023, Some("Q1")) =>
QuarterlyLarRepositoryWrapper("2023", "Q1").getLarRepository.insert(
LarConverter(lar = lar, 2023, isQuarterly = true)
)
case Period(2023, Some("Q2")) =>
QuarterlyLarRepositoryWrapper("2023", "Q2").getLarRepository.insert(
LarConverter(lar = lar, 2023, isQuarterly = true)
)
case Period(2023, Some("Q3")) =>
QuarterlyLarRepositoryWrapper("2023", "Q3").getLarRepository.insert(
LarConverter(lar = lar, 2023, isQuarterly = true)
)
case Period(2024, Some("Q1")) =>
QuarterlyLarRepositoryWrapper("2024", "Q1").getLarRepository.insert(
LarConverter(lar = lar, 2024, isQuarterly = true)
)
case Period(2024, Some("Q2")) =>
QuarterlyLarRepositoryWrapper("2024", "Q2").getLarRepository.insert(
LarConverter(lar = lar, 2024, isQuarterly = true)
)
case Period(2024, Some("Q3")) =>
QuarterlyLarRepositoryWrapper("2024", "Q3").getLarRepository.insert(
LarConverter(lar = lar, 2024, isQuarterly = true)
case Period(submissionId.period.year, None) => YearlyLarRepositoryWrapper(submissionId.period.year.toString).getLarRepository.insert(
Copy link
Contributor

Choose a reason for hiding this comment

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

Does using submissionId.period.year in the case work when you test it? I thought just declaring a new variable year would work here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this works, on another unit test ticket I'm going to refactor this a bit to demonstrate that. I think we should stick to making this depend on the submissionId for now since this line is a part of reading from that.

LarConverter(lar, submissionId.period.year)
)
case Period(submissionId.period.year, Some(submissionId.period.quarter)) =>
QuarterlyLarRepositoryWrapper(submissionId.period.year.toString, submissionId.period.quarter).getLarRepository.insert(
LarConverter(lar = lar, submissionId.period.year, isQuarterly = true)
)
case _ => {
log.error(s"Unable to discern period from $submissionId to insert LAR rows.")
Expand Down
Loading