globalbibletools / gbt

https://interlinear.globalbibletools.com
15 stars 2 forks source link

refactor: remove language and word ids from notes and glosses #394

Closed arrocke closed 6 months ago

arrocke commented 6 months ago

What has changed

Connected Issues

closes #376

QA Steps

Check the following update actions

Post-Deployment

Pertempto commented 6 months ago

When I clicked the "Approve All" button, I got this error in the backend

PrismaClientKnownRequestError:
Invalid `prisma.$executeRaw()` invocation:
Raw query failed. Code: `23505`. Message: `Key ("wordId", "languageId", "timestamp")=(0100100802, 018bba77-bbea-1edf-c38a-d72521f05821, 2024-05-09 05:22:17.78) already exists.`
    at _n.handleRequestError (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:6854)
    at _n.handleAndLogRequestError (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:6188)
    at _n.request (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:5896)
    at async l (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:128:10871)
    at async eval (webpack-internal:///(api)/./pages/api/languages/[code]/glosses/bulk.ts:112:21)
    at async Proxy._transactionWithCallback (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:128:9534)
    at async Object.handler (webpack-internal:///(api)/./pages/api/languages/[code]/glosses/bulk.ts:47:13)
    at async eval (webpack-internal:///(api)/./shared/Route.ts:171:21)
    at async eval (webpack-internal:///(api)/./shared/Route.ts:230:21) {
  code: 'P2010',
  clientVersion: '5.10.2',
  meta: {
    code: '23505',
    message: 'Key ("wordId", "languageId", "timestamp")=(0100100802, 018bba77-bbea-1edf-c38a-d72521f05821, 2024-05-09 05:22:17.78) already exists.'
  }
}

Maybe I did something wrong, I'll try again later. Hope to have more time to review Saturday.

Pertempto commented 6 months ago

There also is a bug where notes aren't always saved. Sometimes they seem to be, and other times they aren't. It seems like it might be more of a frontend bug than a backend bug, so probably should be a separate issue.

Basic steps

  1. Click on a gloss input
  2. Go to the notes tab
  3. Enter a footnote
  4. Click outside the footnote input
    • For some reason the "updated at" text doesn't show up
  5. Click on a different gloss
  6. Click on the original gloss
  7. The footnote is gone
tycebrown commented 6 months ago

There also is a bug where notes aren't always saved. Sometimes they seem to be, and other times they aren't. It seems like it might be more of a frontend bug than a backend bug, so probably should be a separate issue.

I was able to reproduce the bug on the main branch. After a bit of investigating, is seems like the bug first appears after PR #391

arrocke commented 6 months ago

@Pertempto I think I fixed the issue. I was creating duplicate phrases accidentally which caused duplicate entries to be created in the gloss history table for each word, violating the primary key constraint

Pertempto commented 6 months ago

The approve all feature works now

Pertempto commented 6 months ago

I've noticed that the /api/languages/:languageCode/verses/:verseId/glosses requests are taking a LONG time to return. Not sure if it's directly related to this branch. It usually takes more than 5 seconds to return a response, and sometimes as much as 25 seconds. Do we have an inefficient query?

Pertempto commented 6 months ago

It looks like the notes bug was fixed in a separate PR. Were you going to merge that into this branch? I can't really test footnotes or translator notes with that bug occurring.

Pertempto commented 6 months ago

Hmmm I was testing a bit more and somehow I triggered another 500 error with the approve all feature.

The key error seems to come from the history entry insert.

Trying to find steps to reproduce

Backend Error

[
  { wordId: '4300100201', gloss: 'Este', state: 'APPROVED' },
  { wordId: '4300100202', gloss: 'era', state: 'APPROVED' },
  { wordId: '4300100203', gloss: 'en el', state: 'APPROVED' },
  { wordId: '4300100205', gloss: 'con', state: 'APPROVED' },
  { wordId: '4300100206', gloss: '-', state: 'APPROVED' },
  { wordId: '4300100206', gloss: '-', state: 'APPROVED' }
]
PrismaClientKnownRequestError:
Invalid `prisma.$executeRaw()` invocation:
Raw query failed. Code: `23505`. Message: `Key ("wordId", "languageId", "timestamp")=(4300100206, 018bba77-bbea-1edf-c38a-d72521f05821, 2024-05-11 08:58:09.338) already exists.`
    at _n.handleRequestError (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:6854)
    at _n.handleAndLogRequestError (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:6188)
    at _n.request (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:123:5896)
    at async l (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:128:10871)
    at async eval (webpack-internal:///(api)/./pages/api/languages/[code]/glosses/bulk.ts:116:21)
    at async Proxy._transactionWithCallback (C:\Users\addis\SoftwareProjects\gloss-translation\gloss-translation\node_modules\@prisma\client\runtime\library.js:128:9534)
    at async Object.handler (webpack-internal:///(api)/./pages/api/languages/[code]/glosses/bulk.ts:47:13)
    at async eval (webpack-internal:///(api)/./shared/Route.ts:171:21)
    at async eval (webpack-internal:///(api)/./shared/Route.ts:230:21) {
  code: 'P2010',
  clientVersion: '5.10.2',
  meta: {
    code: '23505',
    message: 'Key ("wordId", "languageId", "timestamp")=(4300100206, 018bba77-bbea-1edf-c38a-d72521f05821, 2024-05-11 08:58:09.338) already exists.'
  }
}

Request Payload

{
  "data": {
    "4300100201": {
      "gloss": "Este",
      "state": "APPROVED"
    },
    "4300100202": {
      "gloss": "era",
      "state": "APPROVED"
    },
    "4300100203": {
      "gloss": "en el",
      "state": "APPROVED"
    },
    "4300100205": {
      "gloss": "con",
      "state": "APPROVED"
    },
    "4300100206": {
      "gloss": "-",
      "state": "APPROVED"
    }
  }
}

It seems strange that the entry for 4300100206 is in the patchedGlosses array TWICE

Pertempto commented 6 months ago

I haven't been able to make it happen again, glossed several more verses using approve all with no issue. It seems like the double entry in the array is some sort of bug though.

arrocke commented 6 months ago

I've noticed that the /api/languages/:languageCode/verses/:verseId/glosses requests are taking a LONG time to return. Not sure if it's directly related to this branch. It usually takes more than 5 seconds to return a response, and sometimes as much as 25 seconds. Do we have an inefficient query?

There are two things going on here:

  1. You will notice poor performance locally because of the prefetching. The dev server only has one thread, so when we crash the server with something like 10 requests at once when prefetching verses, it struggles to keep up. Try setting the prefetch const to 0 in your local environment and compare server times to see what I mean
  2. I did push a commit to main that fixed two issues with the query, one of which seemed to significantly reduce the query time when run in pgadmin. I'm seeing the request take between 1-2 seconds now

Hmmm I was testing a bit more and somehow I triggered another 500 error with the approve all feature.

I'm still trying to figure this one out. Haven't been able to get it to fail and haven't found anything in the code that would cause this. Can you confirm that your database is update to date with the latest migrations? I'm not sure that would be it, but I want to rule that out. If we can't reproduce this, I'd like to merge as is given that this feature probably isn't used frequently, and the transaction in the route prevents it from partial succeeding on a failure like this

It looks like the notes bug was fixed in a separate PR. Were you going to merge that into this branch? I can't really test footnotes or translator notes with that bug occurring.

I rebased main into this branch and force pushed. You'll need to delete the branch locally and repull

Pertempto commented 6 months ago

I deleted my local version of this branch and pulled again. I also ran the DB migrations, but my DB was already up to date.

The notes UI wouldn't work at all. I tried typing in notes for one gloss, then clicked on another gloss. I could see the API requests succeeding, but the UI wouldn't update to show the timestamp/username who updated the note. The notes input would keep the same content when I clicked on the other gloss, and when I refreshed the page no notes loaded at all.

I thought maybe there was some issue with my local database, so I tried to reset it using nx run db:prisma migrate reset. That failed too, with errors about pg_restore: error: could not execute query: ERROR: column "wordId" of relation "Footnote" does not exist.

Do notes work for you in this branch? Is it just something wrong with my local environment?

Pertempto commented 6 months ago

Ok it looks like the UI bug exists on main - https://github.com/globalbibletools/gbt/issues/401

We do need to get the seed file updated in this PR

arrocke commented 6 months ago

I think the bug you are seeing is caused by #398. That was already fixed, just not in this branch. I rebased main so that should be good now. Can you try the notes again?

I also updated the seed file.