-
Notifications
You must be signed in to change notification settings - Fork 159
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
batch: tabulate on build/create calls #388
Conversation
@@ -422,15 +430,15 @@ func TestFile__readFromJson(t *testing.T) { | |||
} | |||
batch := file.Batches[0] | |||
batchControl := batch.GetControl() | |||
if batchControl.EntryAddendaCount != 2 { | |||
if batchControl.EntryAddendaCount != 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bkmoovio Shouldn't this be 0
? The ppd-valid.json
file has no addenda records.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why, but Batch.build
calls entryCount++
(which is now 1 + entry.addendaCount()
). That doesn't seem right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Adam,
EntryAddendaCount in the Company/Batch Control Record and File Control Record can be confusing as it is not the AddendaCount for an Entry. It is (for reference page OR108) the tally for each Entry Detail Record processed and each Addenda Record processed, within either the batch or file, as appropriate. For a BatchPPD with one EntryDetail , and no Addenda05, EntryAddendaCount should be 1, If the one EntryDetail has an Addenda05, EntryAddendaCount would be 2. Hopefully that helps . (Clear as Mud)
For the second question:
Prior to removing addendumer change, the code set entryCount as follows:
for i, entry := range batch.Entries {
entryCount = entryCount + 1 + len(entry.Addendum)....
going forward I had added +1 to entryCount for each entry (EntryDetail) + each Addenda property if the property was defined. Addenda02, len(Addenda05), Addenda98, Addenda99. it looks like you created a function for that which is great. I should have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay. Those names are confusing but it makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well that's a good idea.
Codecov Report
@@ Coverage Diff @@
## master #388 +/- ##
==========================================
- Coverage 85.41% 84.78% -0.64%
==========================================
Files 107 107
Lines 6774 8813 +2039
==========================================
+ Hits 5786 7472 +1686
- Misses 712 1059 +347
- Partials 276 282 +6
Continue to review full report at Codecov.
|
@@ -177,7 +179,7 @@ func (f *File) setBatchesFromJSON(bs []byte) error { | |||
if err := json.Unmarshal(bs, &batches); err != nil { | |||
return err | |||
} | |||
// Clear out any nil batchs | |||
// Clear out any nil batchess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
batches
This change gets rid of the requirements to provide a
batchControl
and/orfileControl
objects in the JSON for anach.File
object.Part of: https://github.com/moov-io/moov-io/pull/12