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

Fix Reference lookups aren't logged in database in views.py #724

Merged

Conversation

Frederic-Chopin
Copy link
Contributor

What GitHub issue does this PR apply to?

Resolves #723

What changed and why?

- Fixed issue where reference page lookups were not being stored in the database.
  • Modified concepts function in views.py to call store_lookup_info even when only one language is present.

(If editing Django app) Please add screenshots

Checklist

  • I claimed any associated issue(s) and they are not someone else's
  • I have looked at documentation to ensure I made any revisions correctly
  • I tested my changes locally to ensure they work
  • (If editing Django) I have added or edited any appropriate unit tests for my changes

Any additional comments or things to be aware of while reviewing?

tested locally and now reference page lookup is stored in the web_lookupdata table in the database

@Frederic-Chopin Frederic-Chopin changed the title Fix logging and database storage issues in views.py Fix Reference lookups aren't logged in database in views.py May 23, 2024
This change removes an unnecessary blank line in the views.py file to improve code readability and adhere to code style guidelines.
@geekygirlsarah
Copy link
Member

Such a simple fix! I told myself I'd go dig after work and you beat me to it!

How would you feel about making some unit tests that call the compare and reference pages and ensure it logs a visit? (You can say no since it's easy to call the page but checking the storage of it is a bit more complicated.)

@Frederic-Chopin
Copy link
Contributor Author

Hi Sarah, sure, I can try adding some unit tests over the weekend!

@geekygirlsarah
Copy link
Member

@Frederic-Chopin Are you still working on this? If not, I can merge and maybe split off the tests as another issue. Let me know!

@Frederic-Chopin
Copy link
Contributor Author

Hi Sarah, sorry for the late reply. I think it's best to split the test as another issue, thank you !

@geekygirlsarah
Copy link
Member

I made #725 for it. If you want to work on that, go claim it. If not, I may get to it soon.

Thanks for working on this! I'll merge it in.

@geekygirlsarah geekygirlsarah merged commit 7428ebe into codethesaurus:main Jun 28, 2024
5 checks passed
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.

Bug: Reference lookups aren't logging in database
2 participants