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

new approach in public-adventures #5431

Draft
wants to merge 57 commits into
base: main
Choose a base branch
from
Draft

new approach in public-adventures #5431

wants to merge 57 commits into from

Conversation

hasan-sh
Copy link
Contributor

@hasan-sh hasan-sh commented Apr 17, 2024

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 and public-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 value en_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 and date_adventure_id. As you can expect, the field_value would hold the info that we retrieve from the filters table. For example, level_lang_2_ar where level_lang is the field and 2_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.

NB: We need to run the scripts in the migration_scripts on alpha and beta.

website/public_adventures.py Outdated Show resolved Hide resolved
website/database.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
@@ -215,9 +215,6 @@ msgstr ""
msgid "adventure_name_invalid"
msgstr ""

msgid "adventure_prompt"
Copy link
Collaborator

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?

create-public-adventures-filters.py Outdated Show resolved Hide resolved
static/js/public-adventures.ts Outdated Show resolved Hide resolved
website/database.py Outdated Show resolved Hide resolved
website/database.py Show resolved Hide resolved
website/tags.py Outdated Show resolved Hide resolved
website/tags.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
website/public_adventures.py Outdated Show resolved Hide resolved
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)
Copy link
Collaborator

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).

Copy link
Collaborator

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.

rix0rrr added a commit that referenced this pull request Jun 18, 2024
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).
@Felienne Felienne marked this pull request as draft June 19, 2024 18:15
@Felienne
Copy link
Member

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)

@hasan-sh hasan-sh removed their assignment Jun 25, 2024
@hasan-sh
Copy link
Contributor Author

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:)

mergify bot pushed a commit that referenced this pull request Jun 25, 2024
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.
hasan-sh added a commit that referenced this pull request Jul 4, 2024
mergify bot pushed a commit that referenced this pull request Jul 4, 2024
The reported filter should be implemented in #5431 before we enable it for super-teachers.
@jpelay jpelay added the backend Issues or PRs related with the backend of Hedy, especially in relation to the website and database. label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issues or PRs related with the backend of Hedy, especially in relation to the website and database.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

💻Public adventures, select the language automatically? 🪲 Public adventures 'no adventures available'
4 participants