-
Notifications
You must be signed in to change notification settings - Fork 2
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
Listen to Argyle Webhooks to check when paystubs are fully sync'd #13
Conversation
# Conflicts: # .env # doc/compliance/rendered/apps/data.logical.pdf
if (data.event === 'paystubs.fully_synced') { | ||
this.fullySynced = true; | ||
|
||
this.formTarget.submit(); |
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.
If the event from Argyle is "fully_synced," submit the form and proceed to the next step!
.argyle-loading { | ||
position: relative; | ||
} | ||
|
||
.argyle-loading::after { | ||
background-color: rgba(0, 0, 0, 0.128); | ||
content: ""; | ||
position: absolute; | ||
width: 100%; | ||
height: 100%; | ||
top: 0; | ||
left: 0; | ||
z-index: 9999; | ||
} |
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.
This looks pretty crappy.
skip_before_action :verify_authenticity_token | ||
|
||
def create | ||
if params['event'] == 'paystubs.fully_synced' |
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.
An enhancement here will be to broadcast to the Paystubs channel if/when Argyle has reached at least 3 months of paystub data (instead of all paystub data).
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.
Accoring to argyle docs either partially_synced or fully_synced can happen, but fully_synced will always happen when it finishes. (Basically it could never emit partially_synced). So we'll have to handle for either/or
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 good, but I would like us to address the vulnerability that allows people to subscribe to websockets for any ID. Although we could address it later, I worry we may forget.
@@ -162,6 +162,8 @@ GEM | |||
nio4r (2.7.1) | |||
nokogiri (1.16.4-arm64-darwin) | |||
racc (~> 1.4) | |||
nokogiri (1.16.4-x86_64-darwin) | |||
racc (~> 1.4) |
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 wonder where this churn is coming from. How many times will this end up getting added to the Gemfile, lol
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.
@tdooner hmmm Yeah I"m not sure. I think when George did the Docker stuff it changed.
Co-authored-by: Tom Dooner <[email protected]>
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.
Nice, thanks for making those security improvements. Couple code style things but this seems mergeable to me as is.
Ticket: https://jiraent.cms.gov/browse/FFS-676