Skip to content
This repository has been archived by the owner on Sep 13, 2021. It is now read-only.

Refactor and improve discussion suggestions #288

Merged
merged 4 commits into from
Feb 12, 2021

Conversation

ruohola
Copy link
Member

@ruohola ruohola commented Feb 11, 2021

https://trello.com/c/CTkatfAG/880-show-own-school-first-in-discussion-suggestions

Main reason for this was to keep the most important content suggestions
first. E.g. the user's own school should always be the first school that
we're suggesting.

Changed

  • Improve the relevancy of discussion suggestions.

@ruohola ruohola requested a review from blomqma February 11, 2021 13:46
@ruohola ruohola force-pushed the improve-discussion-suggestions branch from 296f8d8 to df12398 Compare February 11, 2021 13:46
@ruohola ruohola changed the title Refactor and improve resolve_discussion_suggestions Refactor and improve discussion suggestions Feb 11, 2021
@ruohola ruohola force-pushed the improve-discussion-suggestions branch 2 times, most recently from 33602be to 05ae271 Compare February 11, 2021 13:49
Comment on lines +127 to +130
if isinstance(order_by, str):
raise TypeError(
"`order_by` should be an iterable of strings, and not a string."
)
Copy link
Member Author

@ruohola ruohola Feb 11, 2021

Choose a reason for hiding this comment

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

Tä eksplisiittinen tyypin tsekkaus on siks, että str on validi Iterable[str] tyyppi, mutta tässä se kärähtäis tosi pahasti: .order_by("grouped_ordering", *order_by) kun jos order_by olis vaikka "foobar" ni päädyttäis ajamaan query .order_by("grouped_ordering", "f", "o", "o", "b", "a", "r") joka olis lievästi sanottuna mielenkiinosta, ja se error message siitä ei olis kovin selkeä.

Pelkillä tyyppihinteillä ei oo mahdollista määritellä tätä rajotusta: python/typing#256

@ruohola ruohola force-pushed the improve-discussion-suggestions branch from 05ae271 to 64b6b43 Compare February 11, 2021 14:07
@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #288 (da7ec8d) into develop (0c257f1) will increase coverage by 0.18%.
The diff coverage is 90.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #288      +/-   ##
===========================================
+ Coverage    95.96%   96.15%   +0.18%     
===========================================
  Files           78       78              
  Lines         2629     2627       -2     
  Branches       169      167       -2     
===========================================
+ Hits          2523     2526       +3     
+ Misses          68       66       -2     
+ Partials        38       35       -3     
Impacted Files Coverage Δ
skole/utils/shortcuts.py 95.34% <77.77%> (-4.66%) ⬇️
skole/schemas/comment.py 98.88% <100.00%> (+7.13%) ⬆️
skole/schemas/gdpr.py 96.82% <100.00%> (ø)
skole/schemas/suggestions.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c257f1...da7ec8d. Read the comment docs.

@ruohola ruohola force-pushed the improve-discussion-suggestions branch 3 times, most recently from 5b9d593 to 95e2cf7 Compare February 11, 2021 16:20
The queries get formatted just fine to be honest.
Main reason for this was to keep the most important content suggestions
first. E.g. the user's own school should always be the first school that
we're suggesting.
@ruohola ruohola force-pushed the improve-discussion-suggestions branch from 95e2cf7 to da7ec8d Compare February 12, 2021 12:37
@ruohola ruohola merged commit 2e1b87a into develop Feb 12, 2021
@ruohola ruohola deleted the improve-discussion-suggestions branch February 12, 2021 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants