mdwiltfong / secret-santa

https://codecademy-secret-santa-2.fly.dev/
3 stars 6 forks source link

Workflow needs to use prisma scheme from main #18

Open mdwiltfong opened 11 months ago

mdwiltfong commented 11 months ago

The current workflow uses the prisma schema within the PR. As a result, theoretically someone could rewrite the schema, and if it goes unnoticed it could be merged into main. Particularly if the rewritten schema passes the already existing tests. Here is a possible workflow to circumnavigate this:

name: Pull Request Workflow

on: [pull_request]

jobs:
  check-schema:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout main branch
      uses: actions/checkout@v2
      with:
        ref: 'main'
    - name: Save main branch schema
      run: cp path/to/schema.prisma schema-main.prisma

    - name: Checkout PR branch
      uses: actions/checkout@v2
      with:
        ref: ${{ github.head_ref }}

    - name: Compare schema with main
      run: |
        if ! diff schema-main.prisma path/to/schema.prisma; then
          echo "Schema changes detected!"
          exit 1
        fi

  test:
    needs: check-schema
    runs-on: ubuntu-latest
    steps:
    - name: Set up Node.js
      uses: actions/setup-node@v2
      with:
        node-version: '14'

    - name: Install Dependencies
      run: npm install

    - name: Run Tests
      run: npm test

  update-schema:
    if: github.event.label.name == 'schema-change'
    needs: [check-schema]
    runs-on: ubuntu-latest
    steps:
    - name: Checkout PR branch
      uses: actions/checkout@v2
      with:
        ref: ${{ github.head_ref }}

    - name: Update Test Database Schema
      run: |
        # Your command to update the test database schema
        npm run update-schema
DigoBaiocchi commented 10 months ago

Hey Michael!

I'm not very familiar with prisma but I would like to learn more about it. Is it ok if I could have a go on this issue?

mdwiltfong commented 10 months ago

Hey @DigoBaiocchi !

Absolutely! Although this issue is involves Prisma, it will probably focus more on the current GitHub Actions the repo is using. Nonetheless it would really help out, because it's really easy to botch the schema in the main branch if i'm not careful with what's being merged. šŸ™šŸ¼

mdwiltfong commented 10 months ago

Hey @DigoBaiocchi ! Just wanted to check in and see if you're still working on this?

DigoBaiocchi commented 9 months ago

Hey @mdwiltfong, yes I am. Sorry for not getting back to you earlier about this. I have a question about what the workflow ultimately wants to accomplish. I'm starting to think that I was overthinking this issue lol.

So you want the workflow to check if the prisma schema is different and if it is different you want to update the index.ts file located in the prisma folder and re run the tests?

mdwiltfong commented 9 months ago

No worries! We can always try to solve this together during the js sessions.

So when someone is modifying the app, it's super easy to overlook the fact they are modifying the schema. The only net are the tests, and it's totally possible to modify the schema in such a way that the tests pass, despite the changes being undesirable.

I think a good way to circumnavigate this, is to program the workflow to use the schema from main to test any contributions. This way, contributors will have to communicate their schema changes to get it approved. Basically the idea is to protect main as much as possible.

In the event there is a desired change to the schema, we can label the workflow so that it uses the schema in the contributor's PR.