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

Show count over 1000 gives some issues. #5240

Closed
wimpyrbx opened this issue Sep 20, 2018 · 11 comments
Closed

Show count over 1000 gives some issues. #5240

wimpyrbx opened this issue Sep 20, 2018 · 11 comments

Comments

@wimpyrbx
Copy link
Contributor

wimpyrbx commented Sep 20, 2018

When you have db with more than 1000 shows the following issues arise:

  • displayShow page does not display the shows that are above 1000 count (when sorted "alphabetically")

    • so for example if "Still Game" is your #1000 show the show "The Young Ones" does not show data on displayShow page, but the show "A Touch of Cloth" does work.
  • Also an issue is the dropdown box on displayShow does not render all shows, but rather stops at 1000 entries.

When I found what would be show #1001 in the list and removed the #1000 the next show in the list (#1001) took #1000s place in the dropdown. I tried this 4 times by removing of the of shows that was in the list. For each I removed the next in line after #1000 would be visible and displayShow worked.

The index page does however display all show entries if > 1000 in total.
If this issue is affecting other aspects of the medusa enviroment I am unsure as I noticed this today.

Tested in both chrome and firefox.
No visible bugs noted in console or log files as far as I can see.
No errors reported inside medusa error reporting page.

@triage-new-issues triage-new-issues bot added the triage Untriaged issue label Sep 20, 2018
@wimpyrbx
Copy link
Contributor Author

i would like to note that this was not the case before i updated last which might be a week or so ago.

@sharkykh
Copy link
Contributor

sharkykh commented Sep 20, 2018

It's currently limited to getting a maximum of 1000 shows per call.
It's hardcoded on _get_limit (last code block below) but I think that number is just arbitrary.

$store.dispatch('getShows');

getShows(context, shows) {
const { commit, dispatch } = context;
// If no shows are provided get the first 1000
if (!shows) {
const params = {
limit: 1000
};
return api.get('/series', { params }).then(res => {

def _get_limit(self, default=20, maximum=1000):
try:
limit = self._parse(self.get_argument('limit', default=default))
if limit < 1 or limit > maximum:
self._raise_bad_request_error('Invalid limit parameter')
return limit
except ValueError:
self._raise_bad_request_error('Invalid limit parameter')

@wimpyrbx
Copy link
Contributor Author

thanks. hopefully possible to fix in later update. i'll manually edit that file now for myself.

@wimpyrbx
Copy link
Contributor Author

changing those two to 2000 instead of 1000 did not fix the problem. the pyc files generate by themselves when medusa runs them?

@sharkykh
Copy link
Contributor

the pyc files generate by themselves when medusa runs them?

when you restart Medusa, yes

but you need to also update the frontend code


and then you need to compile it using yarn build in themes-default/slim
you need to have Node.js installed to do that
https://github.com/pymedusa/Medusa/wiki/Themes#make-changes-to-existing-themes

@wimpyrbx
Copy link
Contributor Author

nodejs is installed. just getting this.
will wait for an update.

image

@OmgImAlexis
Copy link
Collaborator

@sharkykh we could use vuex-persistedstate and keep the shows array in local storage and just update on load? It would mean we'd have cached results for a few seconds but I think that should be okay..?

@wimpyrbx
Copy link
Contributor Author

is a limiter here really needed? is it put in place due to performance? would it be a lot slower if limit set to 2000 or 3000?

on the other hand, if a limiter is really needed here. why not just put "number of shows in database"-count as the limit?

i know it's low priority for the project as it's set to 3. Low, and I'm not the average user.. but Medusa in it's current state is unusable for me so would love a fix :)

@OmgImAlexis
Copy link
Collaborator

@wimpyrbx want to try something for me? Make these changes and let me know if it loads okay. I'll make a PR when I get a chance.

──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: medusa/server/api/v2/base.py
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ base.py:321 @ class BaseRequestHandler(RequestHandler):
        except ValueError:
            self._raise_bad_request_error('Invalid page parameter')

    def _get_limit(self, default=20, maximum=1000):
    def _get_limit(self, default=20, maximum=10000):
        try:
            limit = self._parse(self.get_argument('limit', default=default))
            if limit < 1 or limit > maximum:
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: themes-default/slim/src/store/modules/shows.js
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ shows.js:81 @ const actions = {
     * Get shows from API and commit them to the store.
     *
     * @param {*} context - The store context.
     * @param {ShowParameteres[]} shows - Shows to get. If not provided, gets the first 1000 shows.
     * @param {ShowParameteres[]} shows - Shows to get. If not provided, gets the first 10k shows.
     * @returns {(undefined|Promise)} undefined if `shows` was provided or the API response if not.
     */
    getShows(context, shows) {
        const { commit, dispatch } = context;

        // If no shows are provided get the first 1000
        // If no shows are provided get the first 10k
        if (!shows) {
            const params = {
                limit: 1000
                limit: 10000
            };
            return api.get('/series', { params }).then(res => {
                const shows = res.data;

@wimpyrbx
Copy link
Contributor Author

@OmgImAlexis i did the changes to the two files and restarted medusa. no change.
if i'm supposed to do more than just edit the files above like running yarn or compile or whatever i have no idea how to do that

medariox pushed a commit that referenced this issue Nov 4, 2018
* Fix #5240 by bumping up the API pagination limit

* Update CHANGELOG.md
@sharkykh sharkykh added this to the 0.2.12 milestone Nov 5, 2018
@sharkykh
Copy link
Contributor

sharkykh commented Nov 5, 2018

Fixed on develop: #5623 bumps the limit to 10,000 shows.

@sharkykh sharkykh closed this as completed Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants