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

remove direct SQL db edits #315

Open
7 tasks
josh-chamberlain opened this issue May 31, 2024 · 1 comment
Open
7 tasks

remove direct SQL db edits #315

josh-chamberlain opened this issue May 31, 2024 · 1 comment
Labels
post-v1 For after sunsetting v1 refactor Improve the code without changing the underlying logic

Comments

@josh-chamberlain
Copy link
Contributor

josh-chamberlain commented May 31, 2024

Context

To the extent it's possible, we should use API endpoints to make changes to the database, never direct SQL edits.

From @maxachis here:

Things such as the Google Agency searcher, as designed, would be inserting data directly into the database. And then there are some things, such as user roles, which don't appear to be governed by an API endpoint and so must be modified through direct SQL queries. As a heuristic (for my own sake rather than yours), any SQL string located in a file that isn't in the data-sources-app repository (and isn't a test) would strongly suggest someplace that's modifying the database outside of an endpoint.

Requirements

  • establish an ADR / policy about when we can have SQL in the code, what kinds of exceptions to allow
    • possible exceptions:
      • testing & migrations
      • ?
  • use the data-sources-app-v2 fork; make changes to dev, eventually merge to main
  • locate cases of SQL which changes the database, and list the cases somewhere we can discuss them
    • SQL located in a file outside data-sources-app repo
    • SQL located outside endpoints in data-sources-app repo
    • keywords like INSERT, UPDATE, DELETE, ALTER, CREATE, DROP, TRUNCATE
  • identify which endpoints + logic would be needed to replace
  • pause, discuss
  • do the thing; replace the SQL with API calls. split it up into chunks/child issues of this one if needed
  • any time SQL affects the db, we should be logging it; especially if it's not through an endpoint

Open questions

  • will we need to worry about auth for new endpoints?
  • when should we do this relative to other work?
@josh-chamberlain josh-chamberlain added the refactor Improve the code without changing the underlying logic label May 31, 2024
@josh-chamberlain josh-chamberlain moved this to Needs Refinement in Open issues & Roadmap May 31, 2024
@josh-chamberlain josh-chamberlain self-assigned this Jun 6, 2024
@josh-chamberlain josh-chamberlain removed their assignment Sep 4, 2024
@maxachis
Copy link
Contributor

@josh-chamberlain I'm marking this as post-v2 because I think :

  1. This will be much easier to navigate once we are freed from Airtable
  2. Similarly, once we're done with the old v1 environment, it'll be easier to develop new endpoints

@maxachis maxachis added the post-v1 For after sunsetting v1 label Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
post-v1 For after sunsetting v1 refactor Improve the code without changing the underlying logic
Projects
Status: Needs Discussion
Development

No branches or pull requests

2 participants