Skip to content

[CCFPCM-0369] pos-amounts#63

Merged
chelsea-EYDS merged 3 commits intomainfrom
CCFPCM-0369
Mar 1, 2023
Merged

[CCFPCM-0369] pos-amounts#63
chelsea-EYDS merged 3 commits intomainfrom
CCFPCM-0369

Conversation

@chelsea-EYDS
Copy link
Contributor

@chelsea-EYDS chelsea-EYDS commented Feb 28, 2023

CCFPCM-0369

Objective:

Parse TDI34 refunds
Script to update current db entries

@chelsea-EYDS chelsea-EYDS changed the title [bugfix] pos-amounts [CCFPCM-0369] pos-amounts Feb 28, 2023
Copy link
Contributor

@vyasworks vyasworks left a comment

Choose a reason for hiding this comment

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

This should be done WHILE PARSING and not as a post parsing DB operation.

@chelsea-EYDS
Copy link
Contributor Author

chelsea-EYDS commented Feb 28, 2023

This should be done WHILE PARSING and not as a post parsing DB operation.

That is correct - this change has been included in the TDI 34 Details class.

The script for updating the old data could run once and then be removed (unless there is a better way to update the old data entries?)

@vyasworks
Copy link
Contributor

This should be done WHILE PARSING and not as a post parsing DB operation.

That is correct - this change has been included in the TDI 34 Details class.

The script for updating the old data could run once and then be removed (unless there is a better way to update the old data entries?)

Correct, we should write migrations for that. (port the controller function to a new migration file)

it is not feasible to run make commands for each of these envs, manually.

@chelsea-EYDS
Copy link
Contributor Author

This should be done WHILE PARSING and not as a post parsing DB operation.

That is correct - this change has been included in the TDI 34 Details class.
The script for updating the old data could run once and then be removed (unless there is a better way to update the old data entries?)

Correct, we should write migrations for that. (port the controller function to a new migration file)

it is not feasible to run make commands for each of these envs, manually.

Okay sounds good. I think I have resolved this, please re-review when you have time.

@@ -1,26 +1,3 @@
export const transactionCode = (code: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Reason for removal?
Was this used before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we first built out the parsing I had included this as a nested field, but then removed in order to keep the flat files "flat" as we had discussed handling any mapping etc after the flat files had been parsed. (This was pre-database, I think we were still using the files in cyber duck at that time ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was no criteria or specs re: the adjustment to the transaction_amt field based on these values at that time.

@chelsea-EYDS chelsea-EYDS merged commit f662600 into main Mar 1, 2023
@chelsea-EYDS chelsea-EYDS deleted the CCFPCM-0369 branch March 22, 2023 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants