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

Lx 301 backsearch feedback backend #53

Merged
merged 7 commits into from
Feb 7, 2024

Conversation

ms0ur1s
Copy link
Contributor

@ms0ur1s ms0ur1s commented Jan 17, 2024

What? Why?

Closes #301

Back end

  • Adds new migration and seeder files for the user_feedback table
  • Updates the following files:
    • queries\database.ts
    • queries\query.ts
    • routes\database.ts
    • validation\validation.ts
      with the relevant additions for submission to the user_feedback table.

user_feedback table columns

  • id
  • question_1
  • question_2
  • question_3
  • question_4
  • user_id (foreign key linked to authenticated user)
  • submission_date

What should we test?

Tested with locally migrated and seeded table. Submissions work in conjunction with a locally running front end, with all columns populating correctly.
As this is my first foray into the LX backend code, and adding a DB table to boot, I'm not sure how the database updates propagate through to the dev server. @rogup and @lin-d-hop I will need some guidance on this, please. 😁

@ms0ur1s ms0ur1s requested a review from rogup January 17, 2024 17:09
@ms0ur1s ms0ur1s self-assigned this Jan 17, 2024
src/queries/database.ts Outdated Show resolved Hide resolved
src/validation.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rogup rogup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ms0ur1s great job on this! The API is looking great and I'm learning a lot about UI design by reading your front end changes.

I've left a few comments on both PRs for possible improvements that I thought about. Let me know if any of it doesn't make sense or you need more info

The BE will also need some unit tests, which we add for all new BE code. There isn't much complicated logic for the feedback route, so the tests will be quite simple. There's more info about how to do this here: https://github.com/DigitalCommons/land-explorer-front-end/wiki/Automated-Testing
We actually want to eventually get into the habit of writing tests alongside the executed code (behaviour-driven development) but since it's your first go at development on the BE, I thought this could wait until next time!

After you've had a look at those docs, let me know if you'd like to pair for a bit to get started with unit tests :)

@ms0ur1s
Copy link
Contributor Author

ms0ur1s commented Jan 23, 2024

Hey @ms0ur1s great job on this! The API is looking great and I'm learning a lot about UI design by reading your front end changes.

I've left a few comments on both PRs for possible improvements that I thought about. Let me know if any of it doesn't make sense or you need more info

The BE will also need some unit tests, which we add for all new BE code. There isn't much complicated logic for the feedback route, so the tests will be quite simple. There's more info about how to do this here: https://github.com/DigitalCommons/land-explorer-front-end/wiki/Automated-Testing We actually want to eventually get into the habit of writing tests alongside the executed code (behaviour-driven development) but since it's your first go at development on the BE, I thought this could wait until next time!

After you've had a look at those docs, let me know if you'd like to pair for a bit to get started with unit tests :)

Thanks @rogup , I'm just limping through this one, as I seem to be doing most of the time, and hoping I get out the other side 😅

Your advice has been great, but as you can see from some of the comments I still need a bit of hand holding and am probably misunderstanding some stuff.

Nice one for the link to unit tests doc, I give that a read. Did you mean we should add a unit test for the user_feedback or that this can wait until the next batch of LX work?

@rogup
Copy link
Collaborator

rogup commented Jan 25, 2024

@ms0ur1s I think you should add the unit tests in this PR, we're trying to do this for all new code. Let me know if you need any help after following the wiki and example tests mentioned in the README :)

@lin-d-hop
Copy link
Contributor

lin-d-hop commented Feb 6, 2024

@ms0ur1s @rogup
How are the unit tests coming along on this?
If you do a skill share can we record it and perhaps add some notes to the wiki?

On second thought..... this time we just dont have budget for unit tests :( 😭

@ms0ur1s ms0ur1s merged commit ad14b22 into development Feb 7, 2024
2 checks passed
@rogup rogup deleted the lx-301-backsearch-feedback-backend branch March 18, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants