#38 - Query performance and Database setup script - Appolin Semegni Fotso #54
Conversation
|
Your Render PR Server URL is https://gaelic-project-pr-54.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cknsnm1rfc9c73b6m78g. |
|
Your Render PR Server URL is https://gaelic-project-pr-54-jzw0.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cknsnmprfc9c73b6m7fg. |
| @@ -14,20 +16,20 @@ CREATE TABLE suggestions ( | |||
| CREATE TABLE user_interactions ( | |||
| id SERIAL PRIMARY KEY, | |||
| sentence_id INTEGER REFERENCES sentences(id), | |||
| selected_suggestion TEXT, | |||
| selected_suggestion INTEGER, | |||
There was a problem hiding this comment.
This change needs the relevant change in the codebase, currently the system will try to save the textual value, which will be converted to 0
There was a problem hiding this comment.
I changed the codebase to reflect the schema. Please let me know if I have taken the right approach.
There was a problem hiding this comment.
Yes, this looks good, Going to verify it locally
|
Rest of the changes look okay for now! |
server/api.js
Outdated
| const randomSentence = oneRow.rows[0].sentence; | ||
| const randomSentenceId = oneRow.rows[0].id; | ||
| // Send randomSentence to frontend; | ||
| res.json([randomSentence, randomSentenceId]); |
There was a problem hiding this comment.
Prefer returning objects:
{
sentence: randomSentence,
id: randomSentenceId
}
…and_query-performance * origin/main: state is not updated if user is authorized submit button is disabled, if auth prompt is fired removed console.log, changed submitCounter to 4 passing props from Home.js add counter and login dialog fire logic add Dialog Popup Window changed navbar depending on user authorization removed redirect if user is no authorized add format on save to be true
There was a problem hiding this comment.
There is a bug when using the "none of the suggestions are useful" and the "original sentence is correct" buttons - the sentence ID is not stored in the database:
cyf=# select * from user_interactions;
id | sentence_id | selected_suggestion | user_provided_suggestion | original_sentence_was_correct
----+-------------+---------------------+--------------------------+-------------------------------
1 | 5 | 1 | |
2 | 4 | 5 | |
3 | 5 | 9 | |
4 | | | | t
5 | | | own correction |
(5 rows)
As you can se rows 4 & 5 are missing the sentenceId values.
Rest of the code looks to be working and is what is expected!
|
Do note I have merged in main, so when you continuw orking make sure to pull this branch first |
|
I did not check those two buttons because they were not part of the ticket and I thought someone else was working on them. |
|
Hi, the ticket didn't mention them as they didn't exist yet. They were added since, and functionality wise they should remain working for any feature we merge afterwards - otherwise we cause a regression and a loss of feature. Note: if you need to redesign their UX to be more in line with how the other buttons work that won't be a problem (specifically the "Correct" button looks a bit too convoluted) |
|
The sentence ID is now stored in the user_interactions table when either of the two buttons is clicked. |
|
Couple minor issues, but I'll address them later, the code looks to be working now and does what is expected |
Naming Rules
Name your PR like this: ISSUENUMBER-TITLE-YOURNAME
Description
I removed the getRandomIndex function in api.js file. I changed how the database is queried for a random sentence by using the ORDER BY random() and LIMIT 1 clauses.
I updated the database schema and the type of data stored in the selected_suggestion column in the user_interactions table.
I updated the codebase to reflect the database schema.
I added a state for the randomTextId and removed unnecessary query on the /save-seggestions endpoint.
Related to
Make sure you include the issue number with a hash sign # so Github can link this PR to the right issue, like this:
Fixes #38
Checklist: