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

Trinity EMS scheduling is now private; scraper needs updating #178

Merged
merged 38 commits into from
Apr 9, 2021

Conversation

pfurbacher
Copy link
Contributor

This update accommodates the "private appointments only" change to the Trinity EMS scheduling page.

  • Adds extraData text from the alert on the page (for the moment, their text is "All appointment types are private, none are available for scheduling.")
  • Adds test and test data (HTML file) covering this new situation.
  • Adds to all tests the following check:
    expect(results).to.have.property("availability");
    expect(results).to.have.property("hasAvailability");

Refactoring the nested functions (moving out of "ScrapeWebsiteData()") requires adding the "private" results parameter to the moved function.
Was yyyy-mm-dd; now m/d/yyyy.
- added saving to s3 and slack messaging
- implemented "service" to make the scraper more testable
- fixed require("awk-sdk") -> s3
- made accumulating availability "more functional"
- clarified confusing code comments
- removed unused "days" var
- fixed typo "writtent" -> "written"
- removed guess as to how to parse slot availability text
Added unit testing for
- no availability
- availability across a number of days in several months

Added a service interface for parsing slot count responses so that can be mocked.

Added an additional "service" provider to the scraper to mock out s3 and slack messaging.
Apparently, "test/bootstrap.js" supplies these in the automated test suite. But
note that without require("./bootstrap.js") in this file, it's not possible for the developer to work with this code in VS Code: both "browser" and "expect" will be marked as undefined. I have found no way to insert bootstrap.js into the launch definition in VS Code.
- Previous version didn't allow testing of s3/slack messaging of the slot count fetch/response. Reworked the pageService interface to facilitate testing.
- Added two expect statements relating to s3/slack messaging .
- Removed extraneous logging.
- Changes to handle the website's change to private appointments only.
- Capture "private appointments only" text; assigned to "extraData" property of results object.
- Added test and sample data (HTML file) to cover this new condition. Tests for presence of "extraData" property in results.
…ests

expect(results).to.have.property("availability");
expect(results).to.have.property("hasAvailability");
@pfurbacher
Copy link
Contributor Author

Oops! Did I do something wrong or are these conflicts expected?

@livgust
Copy link
Owner

livgust commented Mar 25, 2021

@pfurbacher I don't know why you have conflicts in those files; could you perform a git merge from master to make sure everything's up-to-date? I don't think your branch is up to date.

@pfurbacher
Copy link
Contributor Author

Are the Fauna tests failing because of changes I made to TrinityEMS?

@pfurbacher pfurbacher changed the title Trinity EMS scheduling is now private; scraper needs updating #177 Trinity EMS scheduling is now private; scraper needs updating Mar 25, 2021
@livgust
Copy link
Owner

livgust commented Mar 26, 2021

Says it's an authorization issue... @tkjones117 any insight here?

Copy link
Owner

@livgust livgust left a comment

Choose a reason for hiding this comment

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

I tested and this looks good! Just waiting to hear back on auth problems with Fauna and possibly unintended changes to metrics.js.

lib/metrics.js Outdated
item?.availability &&
Object.keys(item.availability).length
) {
if (item?.availability) {
Copy link
Owner

Choose a reason for hiding this comment

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

do you know why this file changed?

Copy link
Contributor Author

@pfurbacher pfurbacher Mar 26, 2021

Choose a reason for hiding this comment

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

I don't. Did I commit a change to metrics.js? I hope not because I haven't ever tinkered with it (I'm not even sure what it's supposed to do). Is it possible it was some artifact of me updating (merge/pull) from upstream, and then ... I have no idea. I'm not very experienced with git.

I can say that locally, when I ran rpm test it was only the Fauna tests which failed. I was surprised that 2 metrics tests failed in the pre-flight checks here. also, I figured that since the Fauna tests were using fake data, there couldn't be any connection to my code update.

Copy link
Owner

Choose a reason for hiding this comment

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

I just checked and we definitely want the metrics.js file to have no diff here - the things that were reverted need to be put back. Once you can do that, @pfurbacher , I can merge!

lib/metrics.js Outdated
item?.availability &&
Object.keys(item.availability).length
) {
if (item?.availability) {
Copy link
Owner

Choose a reason for hiding this comment

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

I just checked and we definitely want the metrics.js file to have no diff here - the things that were reverted need to be put back. Once you can do that, @pfurbacher , I can merge!

@tkjones117
Copy link
Collaborator

@pfurbacher just make sure you have the most recent commits from master on this branch. It looks like you just merged some of Friday's commits, but you still need the commits that have been merged since Friday. Let me know if you want a hand with this.

Copy link
Collaborator

@iann iann left a comment

Choose a reason for hiding this comment

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

This looks good to me. Trinity showing as private, however.

@livgust livgust merged commit 2f91bf8 into livgust:master Apr 9, 2021
@pfurbacher pfurbacher deleted the Trinity-EMS branch April 19, 2021 12:39
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.

4 participants