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

Alert for VVA and VBMS document list retrievals #8276

Merged
merged 24 commits into from Dec 21, 2018

Conversation

hschallhorn
Copy link
Contributor

Resolves #6049

Description

Added error in the document list for when the was never a response from VBMS or VVA. Added a similar warning for when the last response was older than the eFolder cache limit. Reimplemented retrieval timestamps.

hschallh and others added 5 commits December 7, 2018 11:38
* Remove shortcut to return null on nil manifest timestamp values
  that kept error messages from displaying
* Add check for nil VBMS manifest timestamp

References #6049
* Add check for stale manifests
* Add warning alert for stale manifests
* Rename alert component
* Correctly parse manifest time strings
* Add test for _not quite_ so old manifest retrieval time

Still needs copy from design on verbiage
References #6049
@ghost ghost assigned hschallhorn Dec 13, 2018
@ghost ghost added the In-Progress label Dec 13, 2018
@va-bot
Copy link
Collaborator

va-bot commented Dec 13, 2018

1 Warning
⚠️ PR is classed as Work in Progress

Generated by 🚫 Danger

Copy link
Contributor

@mdbenjam mdbenjam left a comment

Choose a reason for hiding this comment

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

Might also be good to update vbms.rb in our initializers to have something like:

if ApplicationController.dependencies_faked?
  VBMSService.manifest_vbms_fetched_at = Time.zone.now
  VBMSService.manifest_vva_fetched_at = Time.zone.now
end

So that our default dev data is not to show the big red alert.

* Update the format of manifest retrieval timestamp timezones to
  offests rather than abbreviations so they can be parsed by
  moment on the front end
@hschallhorn
Copy link
Contributor Author

@mdbenjam Timezoning issues fixed! Timestamps still show the abbreviated timezone, but the timezone offset is also passed up to be parsed by moment for when time calculations are needed.

Copy link
Contributor

@mdbenjam mdbenjam left a comment

Choose a reason for hiding this comment

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

LGTM, do you mind just changing the copy where Sneha and I commented?

@hschallhorn hschallhorn added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Dec 21, 2018
@ghost ghost assigned va-bot Dec 21, 2018
@va-bot va-bot merged commit b4aa4f6 into master Dec 21, 2018
@va-bot va-bot deleted the hschallhorn/reader/6049-vbms-vva-download-error-alert branch December 21, 2018 21:50
@ghost ghost removed the In-Progress label Dec 21, 2018
@anyakhvost anyakhvost changed the title [WIP] Alert for VVA and VBMS document list retrievals Alert for VVA and VBMS document list retrievals Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants