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

Ensy 172 add functionality and tests to lambda #10

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

adamshire123
Copy link
Contributor

@adamshire123 adamshire123 commented Aug 4, 2023

What does this PR do?

Adds functionality and associated tests for transforming the bursar fine and fee data exported from Alma into the format required for uploading to SFS.

  • When invoked with an event containing the job name and job id matching a file in the SOURCE_BUCKET, with the correct SOURCE_PREFIX, retrieve the file.
  • Convert the file to a .csv according to business rules specified by IDLA and SFS (student financial services)
  • Put the transformed file in an s3 bucket where it can be picked up by IDLA staff and manually uploaded to SFS via webform.
  • Return the key of the transformed file (we might return something different once we determine how MIT libraries staff will access the transformed file).

Helpful background context

This lambda will be invoked by a step function following our pattern of Alma job-end webhook -> webhook lambda -> step function -> this lambda

The data in the webhook POST request from Alma doesn't contain enough information for our lambda to retrieve the correct object from s3 directly. Alma names the export file [bursar transfer job name]-[job id]-[timestamp].xml. The POST data doesn't contain the timestamp, so we have to first get a list of filenames in our source location, filtered based on the job_name and job_id from the webhook POST data. There should only be one matching filename because the job_id is unique.

We are still waiting for confirmation on how to calculate some of the values in the transformed file. see https://mitlibraries.atlassian.net/browse/ENSY-182

We are currently putting the transformed file in the sftp bucket under a different path, this may change depending on the outcome of https://mitlibraries.atlassian.net/browse/ENSY-181

How can a reviewer manually see the effects of these changes?

see readme for local testing instructions

Includes new or updated dependencies?

YES

What are the relevant tickets?

https://mitlibraries.atlassian.net/browse/ENSY-172

Developer

  • All new ENV is documented in README (or there is none)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

@adamshire123 adamshire123 force-pushed the ENSY-172-add-functionality-and-tests-to-lambda branch from 911e479 to cd3fc60 Compare August 4, 2023 17:04
@adamshire123 adamshire123 force-pushed the ENSY-172-add-functionality-and-tests-to-lambda branch from cd3fc60 to fc56e5b Compare August 4, 2023 17:06
@adamshire123 adamshire123 marked this pull request as ready for review August 4, 2023 17:09
Copy link

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

Works as expected, just a few suggestions

lambdas/bursar_transfer.py Show resolved Hide resolved
lambdas/bursar_transfer.py Show resolved Hide resolved
lambdas/bursar_transfer.py Show resolved Hide resolved
lambdas/bursar_transfer.py Outdated Show resolved Hide resolved
lambdas/bursar_transfer.py Outdated Show resolved Hide resolved
lambdas/bursar_transfer.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
* add doc string
* add error message
* update readme re: aws creds req. for testing
* fix typos
Copy link

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

One non-blocking comment that I defer to you on whether it is useful or not

lambdas/bursar_transfer.py Show resolved Hide resolved
@adamshire123 adamshire123 merged commit b5959f3 into main Aug 8, 2023
2 checks passed
@adamshire123 adamshire123 deleted the ENSY-172-add-functionality-and-tests-to-lambda branch August 18, 2023 15:32
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