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

Airtable Polling #10

Merged
merged 18 commits into from
Apr 2, 2020
Merged

Airtable Polling #10

merged 18 commits into from
Apr 2, 2020

Conversation

alexquick
Copy link
Contributor

@alexquick alexquick commented Apr 1, 2020

Polling

Creates a new worker that knows how to periodically poll Airtable. This Relies on 2 fields in the Airtable table and provides a 3rd [1]:

  1. Meta: Stores the last values for the row. This is used to determine what has changed and lets you see what the changes were.
  2. Last Modified: Set up to be an automatic Last Modified field on Airtable's side. This lets us easily poll for changed/new rows by keeping track of the max Last Modified we've seen with a small overlap. [2]
  3. Last Processed: Date field that holds the last time this record was processed through the system [2].

The basic flow is to:

  1. Find all records where lastModified > the last observed lastModified minus an overlap. This will return all rows that have changed and some that haven't (that fall in the overlap)
  2. Check to make sure that non-meta columns have actually changed.
  3. Update the record's lastProcessed and meta.lastValues to reflect what the state is currently.
  4. Emit the records that have changed to the caller

Notes

  • You can run it like so:
npm run local:airtable-sync

> mutual-aid-app@ local:airtable-sync /Users/alex/devel/slack-app
> NODE_ENV=dev node ./src/workers/airtable-sync/worker.js

Starting airtable-sync.requests
Found 1 changes in Requests
1 moved from Dispatch Started -> Delivery Needed
Found 0 changes in Requests
  • I didn't end up going with the fancy API for responding to changes. I was playing around with it a bit and I think that we should get some use-cases implemented before we decide what that is going to be. I volunteer to refactor it when that time comes.
  • I did add some helpers to the returned records that let you easily ask them about which fields have changed and what their values are, which makes it a little bit easier at least: https://github.com/crownheightsaid/mutual-aid-app/pull/10/files#diff-5998397ba9505af6ba775adb2b878334R42-R54
  • You can also start the worker in the express process: AIRTABLE_SYNC=5000 npm start
## Slack ![image](https://user-images.githubusercontent.com/57376/78101461-938d3080-73b5-11ea-8f4c-400f86861006.png)

I just sort of ... made something up. The block editor is pretty cool!

  • Ultimately what do we want the slack message to look like?
    We are missing some fields in the airtable that are in the messages that are being manually posted now: name and a public-facing request description (vs. notes).

  • We can rip out the slack part, I wanted to leave it in for the review you you could see the poller being used.

  • Having @display name in the Volunteers base doesn't seem like enough to mention users without problems because names with spaces are handled poorly, so it looks up the slack user based on email address at the moment. This requires the users:read.email scope.

  • If you want to send the message for a single Request you can run:
    node ./src/workers/airtable-sync/newDeliveryRequest.js 1, where in this case 1 is the Request ID of the Request.


[1]
image
[2] These need to be set up as With Time / 24HR / UTC in Airtable
Resolves: #5

src/workers/airtable-sync/worker.js Show resolved Hide resolved
src/workers/airtable-sync/worker.js Outdated Show resolved Hide resolved
const status = record.get(statusFieldName);
const newStatus = record.getPrior(statusFieldName);
console.log(
`${record.get("Request ID")} moved from ${newStatus} -> ${status}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any ideas about how to stem the tide of hardcoded Airtable column names? Everything I can think of feels over-engineered.

Maybe exporting an object out of the airtable module like:

requestFields = {
   requestId: "Request ID",
   status: "Status"
}
. . .  elsehwhere . . .
record.get(requestFields.requestId)

which is, like, verbose but not magical and will let us at least track this a bit easier?

Copy link
Contributor

Choose a reason for hiding this comment

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

definitely agree. That's what we're doing with page names in openHome for the slack app

Copy link
Contributor

Choose a reason for hiding this comment

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

after this afternoon, i definitely see the need for this :)

@alexquick alexquick requested review from mjmaurer and mab253 April 1, 2020 05:02
@alexquick alexquick marked this pull request as ready for review April 1, 2020 05:07
Copy link
Contributor

@mjmaurer mjmaurer left a comment

Choose a reason for hiding this comment

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

Another awesome pull request !!
I love this API, most comments are about documentation

app.json Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
src/workers/airtable-sync/ChangeDetector.js Outdated Show resolved Hide resolved
src/workers/airtable-sync/ChangeDetector.js Outdated Show resolved Hide resolved
src/workers/airtable-sync/ChangeDetector.js Show resolved Hide resolved
const status = record.get(statusFieldName);
const newStatus = record.getPrior(statusFieldName);
console.log(
`${record.get("Request ID")} moved from ${newStatus} -> ${status}`
Copy link
Contributor

Choose a reason for hiding this comment

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

definitely agree. That's what we're doing with page names in openHome for the slack app

src/workers/airtable-sync/worker.js Show resolved Hide resolved
src/workers/airtable-sync/worker.js Outdated Show resolved Hide resolved
src/workers/airtable-sync/ChangeDetector.js Outdated Show resolved Hide resolved
@mjmaurer
Copy link
Contributor

mjmaurer commented Apr 1, 2020

Having @display name in the Volunteers base doesn't seem like enough to mention users without problems because names with spaces are handled poorly, so it looks up the slack user based on email address at the moment. This requires the users:read.email scope.

We can backfill Slack's UID with ChangeDetector

@mjmaurer
Copy link
Contributor

mjmaurer commented Apr 1, 2020

I'd love to get this in and be able to play around with it. if you comment out the slack posting part, i think we should merge this ASAP

@mab253 thoughts?

@alexquick alexquick changed the title Airtable Polling & Sample Slack Airtable Polling Apr 1, 2020
@alexquick
Copy link
Contributor Author

Alrighty, I took out slack and made the changes you suggested. Thanks for the feedback, I was a little foggy when writing some of those comments!

@mjmaurer
Copy link
Contributor

mjmaurer commented Apr 2, 2020

This is an awesome addition and I can't wait to start working with it. Given urgency, I'm going to merge. After chatting with @mab253 I think she would agree.

@mjmaurer mjmaurer merged commit c5bd204 into master Apr 2, 2020
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.

System to poll Airtable for changes
3 participants