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

Local DynamoDB streams Integration #2918

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ on:
branches:
- main

env:
ARC_TABLES_PORT: 8000
ARC_DB_EXTERNAL: true

jobs:
node:
uses: nasa-gcn/.github/.github/workflows/node.yml@main
Expand Down
1 change: 1 addition & 0 deletions __playwright__/circulars/archive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ test.describe('Circulars archive page', () => {
}) => {
// This highlights the search returns limited results because it is looking for exact matches to the string
// This should return many more results and include strings like 230812B
test.slow()
await page.goto('/circulars?query=230812')
const orderedListLocator = page.locator('ol')
const listItemLocator = orderedListLocator.locator('li')
Expand Down
5 changes: 3 additions & 2 deletions __playwright__/circulars/submission.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ import { expect, test } from '@playwright/test'
test.describe('Circulars submission page', () => {
test('posts a submission successfully ', async ({ page }) => {
test.slow()
const timeStamp = Date.now()
await page.goto('/circulars/new')
await page.locator('#subject').clear()
await page
.locator('#subject')
.fill('GRB123456a Submission Playwright Test Subject')
.fill(`GRB123456a Submission Playwright Test Subject ${timeStamp}`)
await page
.getByTestId('textarea')
.fill('GRB Submission Playwright Test Body')
await page.getByRole('button', { name: 'Send' }).click({ timeout: 10000 })
await page.waitForURL('/circulars?index')
await expect(
page.getByRole('link', {
name: 'GRB123456a Submission Playwright Test Subject',
name: `GRB123456a Submission Playwright Test Subject ${timeStamp}`,
})
).toBeVisible()
})
Expand Down
6 changes: 5 additions & 1 deletion app.arc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ dedicatedMasterType t3.small.search
autoSoftwareUpdateEnabled true
offPeakWindowEnabled true

@architect-plugin-dynamodb-local
seedFile seed.json

@plugins
plugin-remix
sandbox-oidc-idp # Sandbox identity provider
Expand All @@ -174,4 +177,5 @@ mission-cloud-platform # Custom permissions for deployment on Mission Cloud Pla
email-outgoing # Grant the Lambda function permission to send email; add email templates.
email-incoming # Enable Lambda handlers for incoming emails
nasa-gcn/architect-plugin-search # Add an AWS OpenSearch Serverless collection.
architect/plugin-lambda-invoker
nasa-gcn/architect-plugin-dynamodb-local
nasa-gcn/architect-plugin-dynamodb-local-streams
24 changes: 19 additions & 5 deletions app/lib/email.server.ts
Copy link
Member

Choose a reason for hiding this comment

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

What do the changes in this file have to do with the DynamoDB simulator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you trigger the lambda, it tries to send an email, which can fail if either the template is not defined (which it isnt on local, thats a sandbox thing I believe) or if the user doesn't have valid credentials and pointing to a matching env (which would be the case for running playwright tests)

Also I am working on resolving the currently failing tests locally, should have that out later

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you're still working on fixing the playwright tests?

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import chunk from 'lodash/chunk'

import { hostname } from './env.server'
import { getEnvBannerHeaderAndDescription } from './utils'
import { getEnvBannerHeaderAndDescription, maybeThrow } from './utils'

Check warning on line 23 in app/lib/email.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/email.server.ts#L23

Added line #L23 was not covered by tests
import { encodeToURL } from '~/routes/unsubscribe.$jwt/jwt.server'

const client = new SESv2Client({})
Expand Down Expand Up @@ -79,6 +79,16 @@
}
}

function maybeThrowSES(e: any, warning: string) {
const formattedWarning = `SES threw ${(e as SESv2ServiceException).name}. This would be an error in production. Since we are in ${process.env.NODE_ENV}, ${warning}.`

Check warning on line 83 in app/lib/email.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/email.server.ts#L82-L83

Added lines #L82 - L83 were not covered by tests

maybeThrow<SESv2ServiceException>(e, formattedWarning, [

Check warning on line 85 in app/lib/email.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/email.server.ts#L85

Added line #L85 was not covered by tests
'ExpiredTokenException',
'NotAuthorizedException',
'UnrecognizedClientException',
])
}

/** Send an email to many recipients in parallel. */
export async function sendEmailBulk({
to,
Expand All @@ -98,7 +108,7 @@
subject,
body: getBody(body),
}),
TemplateName: s.email_outgoing.template,
TemplateName: s.email_outgoing?.template,
},
},
}
Expand All @@ -118,9 +128,13 @@
},
}))
)
await client.send(
new SendBulkEmailCommand({ BulkEmailEntries, ...message })
)
try {
await client.send(

Check warning on line 132 in app/lib/email.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/email.server.ts#L131-L132

Added lines #L131 - L132 were not covered by tests
new SendBulkEmailCommand({ BulkEmailEntries, ...message })
)
} catch (e) {
maybeThrowSES(e, 'email will not be sent')

Check warning on line 136 in app/lib/email.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/email.server.ts#L136

Added line #L136 was not covered by tests
}
})
)
}
Expand Down
11 changes: 11 additions & 0 deletions app/routes/docs.contributing.configuration/route.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -179,5 +179,16 @@ account for GCN production by registering the email address
</Description>
<Default>Kafka producer is disabled</Default>
</tr>
<tr>
<Key>`ARC_TABLES_PORT`, `ARC_DB_EXTERNAL`</Key>
<Description>
Variables for local running DynamoDB. Set the port to `ARC_TABLES_PORT`
to 8000 and `ARC_DB_EXTERNAL` to `true` to use the local DynamoDB docker
image and streams plugins
</Description>
<Default>
Default DynamoDB behavior from Architect and streams are not enabled
</Default>
</tr>
</tbody>
</Table>
Loading
Loading