-
Notifications
You must be signed in to change notification settings - Fork 40
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
17830 - EFT TDI17 Parser #1275
17830 - EFT TDI17 Parser #1275
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1275 +/- ##
==========================================
+ Coverage 91.45% 91.64% +0.18%
==========================================
Files 186 197 +11
Lines 11319 11621 +302
==========================================
+ Hits 10352 10650 +298
- Misses 967 971 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
queue_services/payment-reconciliations/src/reconciliations/eft/eft_base.py
Outdated
Show resolved
Hide resolved
self.record_type = self.content[0:1] | ||
self.validate_record_type(EFTConstants.TRANSACTION_RECORD_TYPE.value) | ||
|
||
self.ministry_code = self.content[1:3].strip() |
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.
Could we include a link to the spec? Or a comment listing the spec?
That way the next person knows where we are building this from?
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.
Added a comment with the spec in eft_base
queue_services/payment-reconciliations/src/reconciliations/eft/eft_record.py
Outdated
Show resolved
Hide resolved
queue_services/payment-reconciliations/src/reconciliations/eft/eft_record.py
Outdated
Show resolved
Hide resolved
We should probably put in a unit test where we use a real file (non programmatically) and it parses the expected result You're using factories to build it which could be incorrect in the first place EG. some of the EJV unit tests do that |
Sounds good I will do this. Update: I took a look at the EJV tests, it seems to manually writes the content with parameterized strings. For the EFT Parser I have included a test sample file and a test to read this file and assert the parsing results. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
I bumped the versions, for pay-api as I added models in relation to EFT processing and for this component. |
@@ -11,6 +11,50 @@ | |||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
|
|||
# TDI17 File Specifications |
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.
Perfect this helps a ton
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.
Looks great, thank you!
Issue #:
bcgov/entity#17830
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-pay license (Apache 2.0).