-
Notifications
You must be signed in to change notification settings - Fork 289
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
new approach in public-adventures #5431
base: main
Are you sure you want to change the base?
Conversation
- remove indexes - return filters by field - return adventure_ids only from indexes
@@ -215,9 +215,6 @@ msgstr "" | |||
msgid "adventure_name_invalid" | |||
msgstr "" | |||
|
|||
msgid "adventure_prompt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this removed on purpose?
website/database.py
Outdated
def get_public_adventures_indexes(self, key): | ||
"""This function returns adventure_ids for a specific index. | ||
E.g., key={"field_value": "lang_en", "date_adventure_id": "..."}""" | ||
result = PUBLIC_ADVENTURES_INDEXES.get_all(key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want query them in reverse, so that the newest adventures are first.
Use the Dynamo feature (reverse=True
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well you know what, I guess order doesn't matter since you're throwing the results into a set
afterwards anyway.
But it could matter, and perhaps should matter, to preserve some sort of ordering and have the newest results on top.
- changed http method to put instead of post - changed the migration scripts accrodingly
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Two tiny changes ported over from #5431: - Remove a duplicate database retrieval when looking up the tags of an adventure. - Sort the tags (when they are saved to the database).
Converting this to a draft, there are a few good ideas in this PR but we will separate them out (no need to do anything more @hasan-sh) |
Sure, it's actually done and I've satisfied all Rico's comments. Will keep an eye and see if anything needs to be clarified or changed as well:) |
Two tiny changes ported over from #5431: - Remove a duplicate database retrieval when looking up the tags of an adventure. - Sort the tags (when they are saved to the database). **How to test** Unsorted tags should be sorted after adding a tag. Otherwise no functional difference.
The reported filter should be implemented in #5431 before we enable it for super-teachers.
Fixes #5368
Fixes #5367
This PR changes the functionality of how we retrieve public adventures. It also encapsulates changes related to creating indexes and filters. As a result, we create two additional tables:
public-adventure-filters
andpublic-adventure-indexes
.The first table creates a combination of filters, creating field and value in each record. The field can, for instnace, be
lang_level
and the valueen_1
. These filters are updated regularly once public adventures are created.The second table creates the indexes so that we get the adventure_ids of all public adventures based on what users filter by. The two main fields are
field_value
anddate_adventure_id
. As you can expect, thefield_value
would hold the info that we retrieve from the filters table. For example,level_lang_2_ar
wherelevel_lang
is the field and2_ar
are the values, respectively. Then we can get the adventure ids of these adventures and list them to the user.Thereby, we can paginate the adventures and have much fewer problems with the overload and download restriction of AWS.