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.

Sign up for GitHub

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

Replace hardcoded keys with the centralized list of Airtable keys #153

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Replace hardcoded keys with the centralized list of Airtable keys #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a286c1
Replace hardcoded keys with the centralized list of Airtable keys
Sep 22, 2020
31f9fd8
Add the remaining raw airtable strings
Sep 29, 2020
6f14a5f
Oops, fixing linting error.
Sep 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/lib/airtable/tables/requests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { merge } = require("lodash");
const _ = require("lodash");
const { airbase } = require("~airtable/bases");
const { airbase } = require("../bases");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For some reason I had to make this a relative path. The tilde path threw an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

very weird! do you remember the error? this should be currently in prod right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, super weird! The version with the tilde path is in prod, which proves that it works. Maybe this is just user error on my part.

Here's the error: Module not found: Can't resolve '~airtable/bases' in '/Users/emmasmith/projects/mutual-aid-app/src/lib/airtable/tables'

I see this when I run npm run local:react. Also when I try to build for prod using npm run build. The Express app runs just fine. It seems that the React side of things is not respecting the moduleAliases.

Clues, advice, guidance most appreciated :)


const requestNotInSlack = (r) => {
const meta = r.get(fields.meta);
Expand Down Expand Up @@ -322,6 +322,8 @@ const fields = (exports.fields = {
neighborhood: "Neighborhood MA-NYC",
householdSize: "Household Size",
forDrivingClusters: "For Driving Clusters",
slackChannel: "slack_channel",
slackTimestamp: "slack_ts",
});
exports.SENSITIVE_FIELDS = [
fields.phone,
Expand Down
13 changes: 7 additions & 6 deletions src/webapp/components/RequestPopup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import HouseholdSizeChip from "./HouseholdSizeChip";
import DrivingClusterChip from "./DrivingClusterChip";
import { daysSinceSlackMessage } from "../helpers/time";
import ClusterMapContext from "../context/ClusterMapContext";
import { fields } from "../../lib/airtable/tables/requests";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here: the absolute path (with or without tilde) didn't work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Possibly this is old news, but after looking over it, it looks like the ~whatever module aliases aren't in use anywhere in the webapp directory... surprising! But I think that at least means that this problem isn't because of any change in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it looks like most of the other scripts in our package.json are requiring module-alias/register which I assume is what's giving other scripts the ability to resolve those aliases. However, I'm not actually sure how to do that in local:react since it runs react-scripts instead of node.


const useStyles = makeStyles((theme) => ({
divider: {
Expand Down Expand Up @@ -70,17 +71,17 @@ const RequestPopup = ({ requests, closePopup }) => {
underline="always"
target="_blank"
>
{meta["First Name"] || ""}
{meta[fields.firstName] || ""}
</Link>
) : (
meta["First Name"]
meta[fields.firstName]
)}
</Typography>

<Typography variant="body1">
{meta["Cross Street #1"]}
{meta[fields.crossStreetFirst]}
{" and "}
{meta["Cross Street #2"]}
{meta[fields.crossStreetSecond]}
</Typography>

<Typography variant="body2">
Expand All @@ -91,9 +92,9 @@ const RequestPopup = ({ requests, closePopup }) => {
</Typography>

<Box className={classes.chipRow}>
<HouseholdSizeChip size={meta["Household Size"]} />
<HouseholdSizeChip size={meta[fields.householdSize]} />

{meta["For Driving Clusters"] && <DrivingClusterChip />}
{meta[fields.forDrivingClusters] && <DrivingClusterChip />}

{meta.slackPermalink ? (
<DaysOpenChip daysOpen={daysSinceSlackMessage(meta.slackTs)} />
Expand Down
6 changes: 3 additions & 3 deletions src/workers/airtable-sync/actions/updateMessageContent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const mappings = {
module.exports = async function updateMessageContent(record) {
/* eslint dot-notation: ["error", { "allowPattern": "^[a-z]+(_[a-z]+)+$" }] */
const meta = record.getMeta();
if (!meta["slack_ts"]) {
if (!requestFields.slackTimestamp) {
return;
}
const existingMessage = await getExistingMessage(
Expand Down Expand Up @@ -69,8 +69,8 @@ module.exports = async function updateMessageContent(record) {
}

await slackapi.chat.update({
channel: meta["slack_channel"],
ts: meta["slack_ts"],
channel: requestFields.slackChannel,
ts: requestFields.slackTimestamp,
text: newContent,
});
};
Expand Down