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

Replace Hardcoded API Endpoints with Environment Variables #31

Open
4 tasks
AyushDharDubey opened this issue Dec 17, 2024 · 7 comments
Open
4 tasks

Replace Hardcoded API Endpoints with Environment Variables #31

AyushDharDubey opened this issue Dec 17, 2024 · 7 comments
Labels
easy enhancement New feature or request

Comments

@AyushDharDubey
Copy link
Collaborator

AyushDharDubey commented Dec 17, 2024

Description

All the utility functions in the project currently use hardcoded API endpoints. For example:

const res = await fetch("http://localhost:3000/api/Tickets", {
  cache: "no-store",
});

Hardcoding URLs can lead to deployment issues when switching between different environments, such as production, staging, or testing. To ensure flexibility and maintainability, these values should be moved to environment variables.


Acceptance Criteria

  • No hardcoded URLs remain in the project.
  • API calls use the process.env.NEXT_PUBLIC_API_URL variable.
  • The README includes updated documentation for environment variable setup.
  • Existing functionality (e.g., data fetching) works as expected in local development environment.
@AyushDharDubey AyushDharDubey added enhancement New feature or request easy labels Dec 17, 2024
@anshkarwasra
Copy link
Collaborator

added these vars to .env file
NEXT_PUBLIC_API_URL = http://localhost:3000/api/Tickets
NEXT_DEPLOYMENT_API_URL =
NEXT_TESTING_API_URL =
NEXT_PUBLIC_API_TICKET_ENDPOINT = /api/Tickets

are implemented them as need in app

@AyushDharDubey
Copy link
Collaborator Author

Hi @anshkarwasra,

adding a .env file directly into the codebase is not a good idea for obvious security reasons. that's why we advise each developer/user to create their own .env.local and feed data into it

we can do one thing: update the SAMPLE.env.local with desired values and update readme to advise users to populate their .env.local likewise

regarding the variables:

  • i believe the value for NEXT_PUBLIC_API_URL should be http://localhost:3000/api/ because, in the future, we might introduce other API endpoints, such as http://localhost:3000/api/getCategories. By maintaining this structure, we can dynamically append individual endpoints (e.g., /Tickets and /getCategories).

  • the variable NEXT_PUBLIC_API_TICKET_ENDPOINT = /api/Tickets is redundant and can be omitted, as it’s already covered by the general API structure.

  • lastly, it would be helpful to include the information regarding NEXT_DEPLOYMENT_API_URL and NEXT_TESTING_API_URL in the README so that developers know how to configure these variables for different environments.

you may go ahead and open a PR whenever you are ready
thankyou

@MadhavDhall
Copy link
Contributor

Hi @anshkarwasra sir, in my opinion there is no need to add domain name in fetch request url because the api is on same domain as is the frontend. Making extra fields in env file is not needed and creating such fields just make extra fields and increase the efforts for contributors and also during hosting.

So imo, the url should be eg- "/api/getCategories" instead of "http://localhost:3000/api/getCategories"

I have tried it and works completely fine.
Kindly tell if there is any issue in this approach?

If you feel the approach is fine kindly assign to me the issue so that I can resolve it.
Thanks

@AyushDharDubey
Copy link
Collaborator Author

Hi @MadhavDhall, thank you for showing your willingness to resolve the issue. I affirm your proposed implementation, please wait until the issue is assigned to you and then you may open a new PR for the issue.

@anshkarwasra, are you still working on this issue? If we do not receive an update from you by tomorrow 2359 hours, we will need to reassign the issue to @MadhavDhall.

@MadhavDhall
Copy link
Contributor

@AyushDharDubey kindly assign me this issue. thanks

@AyushDharDubey
Copy link
Collaborator Author

Hi @MadhavDhall,
there has been a slight change in the protocols,
now no participant can be assigned more than 2 issues at the same time,
you already have 4 issues assigned to you, so please address the issues already assigned to you then we will be able to assign this one to you

hope you understand

@MadhavDhall
Copy link
Contributor

Hi @AyushDharDubey sir kindly assign me this issue as currently 3 issues are assigned to me but 2 of them have been resolved by me and PR raised.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants