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

Fix hubspot form build #592

Merged
merged 6 commits into from
Mar 4, 2025
Merged

Fix hubspot form build #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4bfe12b
Add default ID for hubspot form
Strift Feb 27, 2025
02cec2b
Add default .env
Strift Feb 27, 2025
4f74594
Remove defaults from code, use .env only
Strift Feb 27, 2025
63dfc14
Bump version
Strift Feb 27, 2025
dd5242c
Add test for email subscription
Strift Mar 3, 2025
387b546
Validate env variables before building
Strift Mar 3, 2025
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
3 changes: 3 additions & 0 deletions .env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Hubspot newsletter
REACT_APP_HUBSPOT_PORTAL_ID=25945010
REACT_APP_HUBSPOT_FORM_GUID=991e2a09-77c2-4428-9242-ebf26bfc6c64
11 changes: 11 additions & 0 deletions cypress/e2e/side-panel.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const WAITING_TIME = Cypress.env('waitingTime')

describe(`Right side panel`, () => {
beforeEach(() => {
cy.saveApiTokenCookie()
Expand All @@ -18,4 +20,13 @@ describe(`Right side panel`, () => {
cy.get('button[aria-label="Open Panel"]').click()
cy.get('[data-testid="right-panel"]').should('be.visible')
})

it('Should allow subscribing to the newsletter', () => {
cy.get('input[placeholder="Enter your email"]').type('[email protected]')
Copy link
Member

Choose a reason for hiding this comment

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

Will this trigger the subscription, or is it caught somewhere to not send it in testing environment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This triggers a subscription, but nothing happens when you subscribe with an existing email.

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what you mean by "nothing happens when you subscribe with an existing email"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can see that there are multiple submissions, but the contact is simply "upserted" there is no duplicate contacts created in HubSpot. You can see the forms submissions here: https://app-eu1.hubspot.com/submissions/25945010/form/991e2a09-77c2-4428-9242-ebf26bfc6c64/submissions?redirectUrl=https://app-eu1.hubspot.com/forms/25945010/views/all_forms

Copy link
Member

Choose a reason for hiding this comment

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

Ok I'm wondering if we could have improved it by either providing a fake email or having intercepted the request to avoid spamming Hubspot's API 🤔
Approving for now since it's urgent but we should consider doing this test in a different way 🥲

cy.get('button[aria-label="Subscribe"]').click()
cy.wait(WAITING_TIME)
cy.get('[data-testid="right-panel"]').within(() => {
cy.get('p').should('contain', 'Thanks for subscribing!')
})
})
})
7 changes: 4 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "mini-dashboard",
"version": "0.2.17",
"version": "0.2.18",
Copy link
Member

Choose a reason for hiding this comment

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

Can we create a different PR for the version upgrade? 🙏

Copy link
Member

Choose a reason for hiding this comment

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

We want to be fast on this one since the engine team is waiting + timezone issue between Laurent and us, so I would go with this even if it's not really clean, I agree 😊

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I will keep that in mind for the future.

"private": true,
"dependencies": {
"@meilisearch/instant-meilisearch": "0.24",
Expand All @@ -22,8 +22,9 @@
},
"scripts": {
"version-script": "node version-script.js",
"prestart": "yarn version-script",
"prebuild": "yarn version-script",
"validate-env": "node validate-env.js",
"prestart": "yarn version-script && yarn validate-env",
"prebuild": "yarn version-script && yarn validate-env",
"start": "react-scripts start",
"start:ci": "REACT_APP_MEILI_SERVER_ADDRESS=http://meilisearch:7700 react-scripts start",
"build": "react-scripts build",
Expand Down
6 changes: 5 additions & 1 deletion src/components/NewsletterForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ const NewsletterForm = () => {
required
style={{ marginTop: '0rem' }}
/>
<Button type="submit" disabled={status === 'loading'}>
<Button
type="submit"
disabled={status === 'loading'}
aria-label="Subscribe"
>
{status === 'loading' ? (
<>
<SpinningIcon>
Expand Down
2 changes: 1 addition & 1 deletion src/version/version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.17'
export default '0.2.18'
13 changes: 13 additions & 0 deletions validate-env.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/* eslint-disable no-console */

const requiredEnv = ['REACT_APP_API_URL', 'REACT_APP_ANOTHER_ENV_VAR']

const missingEnv = requiredEnv.filter((env) => !process.env[env])

if (missingEnv.length > 0) {
console.error(
`Missing required environment variables: ${missingEnv.join(', ')}`
)
process.exit(1)
}
console.log('All required environment variables are set.')