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

fix: Update SnapWP environment variables #57

Merged
merged 13 commits into from
Jan 16, 2025
Merged

Conversation

Swanand01
Copy link
Contributor

@Swanand01 Swanand01 commented Jan 15, 2025

What

This PR updates the env variables shown in the admin panel.

Why

Currently snapwp-helper uses outdated env variables.
image

They need to be what's expected in our starter: https://github.com/rtCamp/headless/blob/develop/frontend/examples/nextjs/starter/.env.example

Related Issue(s):

How

The missing variables are added to the VariableRegistry, and the old ones updated.
A commented property is added to the variables, which is used to determine if the variable declaration line is commented in the env file preview shown in the admin panel.

Testing Instructions

Go to GraphQL > SnapWP in the WordPress admin panel.

Screenshots

image

Additional Info

Checklist

  • I have read the Contribution Guidelines.
  • My code is tested to the best of my abilities.
  • My code passes all lints (PHPCS, PHPStan, ESLint, etc.).
  • My code has detailed inline documentation.
  • I have added unit tests to verify the code works as intended.
  • I have updated the project documentation accordingly.

@Swanand01 Swanand01 requested a review from justlevine January 15, 2025 13:01
@justlevine justlevine requested a review from Ta5r January 15, 2025 13:10
@Swanand01
Copy link
Contributor Author

@justlevine / @Ta5r Does this approach look good? If yes, I'll update the tests next.

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

Did a quick first pass re the formatting and to get some clarification about your commented approach.

Pinged @Ta5r to review and assist.

access-functions.php Outdated Show resolved Hide resolved
access-functions.php Outdated Show resolved Hide resolved
src/Modules/Admin.php Outdated Show resolved Hide resolved
src/Modules/EnvGenerator/VariableRegistry.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

Did some cleanup + formatting changes in my last commit, accidentally invalidating @Ta5r 's tests 😬.

Fix those tests (again, apologies) and this should be ready to merge

@Swanand01
Copy link
Contributor Author

Did some cleanup + formatting changes in my last commit, accidentally invalidating @Ta5r 's tests 😬.

Fix those tests (again, apologies) and this should be ready to merge

No worries, we'll fix the tests.

@Swanand01
Copy link
Contributor Author

@justlevine This PR is now ready for a final review/merge.

@justlevine justlevine changed the title Fix: Update env variables shown in the admin panel fix: Update SnapWP environment variables Jan 16, 2025
@justlevine justlevine merged commit 8c2be0a into develop Jan 16, 2025
7 of 8 checks passed
@justlevine justlevine deleted the fix/update-env-vars branch January 16, 2025 13:14
@Swanand01 Swanand01 self-assigned this Jan 17, 2025
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.

3 participants