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

Refactor: Routes for group specific vocab activation #94

Merged
merged 11 commits into from
Feb 26, 2023

Conversation

noctera
Copy link
Member

@noctera noctera commented Sep 25, 2022

Status Type Env Vars Change
✔️ Ready Feature No

Description

This PR will be the basis for the upcoming PR in the Frontend (issued here vocascan/vocascan-frontend#132).
I updated the routes to return groups and vocabs that contain staged vocabs.

  1. Updated GET group route
    GET /api/languagePackage/:languagePackageId/group
    Added query parameter ?onlyStaged=true to only return groups that contain at least one unactivated/staged vocab
    Added query parameter ?onlyActivated=true to only return groups that contain at least one active vocab

  2. Updated GET language package route
    GET /api/languagePackage/
    Added query parameter ?onlyActivated=true to only return language packages that contain at least one group with at least one active vocab

  3. Update GET query route
    GET /languagePackage/:languagePackageId/query
    Add query parameter ?groupId={groupId}&groupId={groupId} to return only vocabs from these group
    Add query parameter onlyStaged=true to return only vocabs that are staged
    Add query parameter onlyActivated=true to return only vocabs that are already activated
    (groupId query parameter can be combined with either onlyStaged or onlyActivated)

  4. Updated GET vocab group
    GET /api/group/:groupId/vocabulary
    Added query parameter ?onlyStaged=true to only return vocabs of a group that are already activated (for custom learning)

Motivation and Context

The activate function was kind of useless, as you could just start a query that throws every inactivated/staged vocabs in a query. You couldn't decide which group you want to add to your daily query. With this PR i want to provide the backend basis to implement group specific vocab activation.

In addition to this this PR will give the basis to implement the upcoming "Custom Learning" functionality (issued here vocascan/vocascan-frontend#126

Screenshots / GIFs (if appropriate):

Checklist

  • I have read the CONTRIBUTING document.
  • I have considered the accessibility of my changes (i.e. did I add proper content descriptions to images, or run my changes with talkback enabled?)
  • I have documented my code if needed

Resolves

#93
vocascan/vocascan-frontend#126
vocascan/vocascan-frontend#132

@noctera noctera added kind/enhancement New feature or request kind/docs Improvements or additions to documentation kind/api priority/medium labels Sep 25, 2022
@noctera noctera added this to the v1.4.0 milestone Sep 25, 2022
@noctera noctera self-assigned this Sep 25, 2022
@noctera
Copy link
Member Author

noctera commented Sep 29, 2022

If have updated all routes, everything should work. I will just do some code cleanup and refactoring
I just have a small topic I want to discuss. With this PR I have built in the functionality for "Custom Learning". Now you will have the option to get every vocab card from one or multiple specific groups in your query. But as far as it is implemented now, this includes unactivated vocabs too, which will be given back. Do you think we should exclude those for custom learning, as they have not been activated yet?

This is just a question for this temporarily setup. I would say we implement the full Custom Learning features later, as this is something like a test drive which will be available because it is made available through another function we built

Copy link
Member

@luwol03 luwol03 left a comment

Choose a reason for hiding this comment

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

Just a minor change, otherwise code looks good.

app/Controllers/QueryController.js Outdated Show resolved Hide resolved
@noctera noctera merged commit bcff51e into experimental Feb 26, 2023
@noctera noctera deleted the activate-refactor branch February 26, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api kind/docs Improvements or additions to documentation kind/enhancement New feature or request priority/medium
Projects
Status: 🚀 Ready
Development

Successfully merging this pull request may close these issues.

2 participants