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

[Not urgent] Add Pagination in Uploads API. #2996

Closed
ujjwalagrawal17 opened this issue Jun 19, 2019 · 31 comments
Closed

[Not urgent] Add Pagination in Uploads API. #2996

ujjwalagrawal17 opened this issue Jun 19, 2019 · 31 comments

Comments

@ujjwalagrawal17
Copy link
Member

in Some courses like https://outreachdashboard.wmflabs.org/courses/WMFr/EDITATHONS_2017/uploads.json ,the uploads API is taking a lot of time to give response [this course has around 500 images]. We can implement pagination in the API to give results by using parameter page_number.

@ujjwalagrawal17 ujjwalagrawal17 changed the title Add Pagination in Uploads API. [Not urgent] Add Pagination in Uploads API. Jul 1, 2019
@ANURADHAJHA99
Copy link

@ujjwalagrawal17 I would like to work on this issue. KIndly assign this to me.

@ujjwalagrawal17
Copy link
Member Author

@ANURADHAJHA99 feel free to start working on it. We will use this in the Android app after it's ready.

@ANURADHAJHA99
Copy link

@ujjwalagrawal17 KIndly help me if I am wrong. But I visited the website https://outreachdashboard.wmflabs.org/courses/WMFr/EDITATHONS_2017/uploads and the uploads section already had the pagination. Can you kindly explain how should I start with the issue? Thank you very much.

@ujjwalagrawal17
Copy link
Member Author

I guess the support in web version is added but in https://outreachdashboard.wmflabs.org/courses/WMFr/EDITATHONS_2017/uploads.json API it's not added. We need to add the required parameters to make it work.

@ujjwalagrawal17
Copy link
Member Author

We are consuming the API from Android App and in some courses we are getting very slow response.

@ANURADHAJHA99
Copy link

@ujjwalagrawal17 Thank you very much. however, can you give me an idea of how should I start with the issue, what steps should I start taking, what files do I need to change? It will be very helpful.

@ujjwalagrawal17
Copy link
Member Author

@ragesoss might be able to help you in that. I am not familiar with the website codebase.

@ANURADHAJHA99
Copy link

@ragesoss Can you please guide me through the following issue? Also, Thank you very much @ujjwalagrawal17 for helping.

@ragesoss
Copy link
Member

The uploads endpoint is CoursesController#uploads, which basically just finds the course and then immediately renders the corresponding view, app/views/courses/uploads.json.jbuilder.

The most similar place where you can find an example of pagination already in use is CampaignsController, which sets the page from the query parameters, and then uses the will_paginate gem to get the page-based query results in the 'articles' and 'programs' views. The same strategy should work here, but for CoursesController#uploads we would need to either only paginate if the page param is there (so that the web version maintains the current behavior), or also update the website frontend to use the pagination. Currently, we just have client-side pagination on the web, so it requests and stores the full set of uploads data, and then only renders one page at a time but does not make additional requests to the server.

@ANURADHAJHA99
Copy link

@ragesoss Can you please navigate me to the CampaignsController code to understand the context? I am having some difficulty in understanding the codebase.

@ragesoss
Copy link
Member

CampaignsController is in app/controllers/campaigns_controller.rb. It's responsible for handling web requests to main 'campaigns' views, such as https://dashboard.wikiedu.org/campaigns/fall_2018/programs

If you're unfamiliar with Ruby on Rails, I suggest starting by exploring the Rails documentation a bit to see how models, views, and controllers interact.

@ANURADHAJHA99
Copy link

@ragesoss we have to find the page param from the URL and use the will_paginate gem if the param is present? am I following the correct way? Thank you for the feedback.

@ragesoss
Copy link
Member

Yes, I think so.

@ANURADHAJHA99
Copy link

ANURADHAJHA99 commented Oct 25, 2019

Going by the campaign_controller.rb, I came across various dependencies, What are the required dependencies to make the course-controller, that will extract the URL and then the page param from the dependencies?
Screenshot from 2019-10-25 22-18-02

@ragesoss
Copy link
Member

Rails provides all request parameters via params, which is basically just a hash.

This controller uses the set_page method to set an instance variable from that param, and the instance variable can then be used in the view template that gets rendered.

@ANURADHAJHA99
Copy link

Am I supposed to make another view for the uploads, just like article and programs in the campaign controller, because I have set the params of the page in course-controller but I cannot find uploads.haml which is used to render as a view?

@ragesoss
Copy link
Member

For courses controller, it uses a json jbuilder view template instead of an html haml template. This app/view/courses/uploads.json.builder.

@tafodinho
Copy link

Hi, @ragesoss. Was this issue successfully solved?? if not I would love to get right on itas my first contribution.

@ANURADHAJHA99
Copy link

@tafodinho I had various problems while solving the following issue. You can refer to the pull requests that I made, to understand the errors. Thank you for taking the following issue. I am sorry I could not work due to the lack of knowledge and time.

@tafodinho
Copy link

okay @ANURADHAJHA99 let me take a look right now

@ciphereck
Copy link

hi @tafodinho are you still working on this?
@ragesoss is the issue still open?

@ragesoss
Copy link
Member

ragesoss commented Feb 3, 2020

Yes, it's still open.

@ciphereck
Copy link

@ragesoss okay I'm starting to work on it

@ciphereck
Copy link

I'm still working on it. Just getting to know the codebase and setting up the development environment. Reached till populating the database.

@ciphereck
Copy link

@ragesoss
I have an issue
My http://localhost:3000/courses/ab/ab/uploads.json returns {"course":{"uploads":[]}}
Idk how to populate data in upload array to check my pagination
Can you please tell me how to populate data in this uploads array in the database?

@ciphereck
Copy link

ciphereck commented Feb 7, 2020

@ujjwalagrawal17 @ragesoss
Can you please tell me how many entries we want on a page?
For 10 entries, the INFO says it completed in 10ms
and for 193 entries, it says it completed in 40ms

I think 10 is fine for android because android has less screen space (shorter screen size).

@ujjwalagrawal17
Copy link
Member Author

Yes 10 is fine. If you can add another variable to make it configurable, it will be great.

@ciphereck
Copy link

Sure, I can add another variable.

@ciphereck
Copy link

I added another variable and updated the PR. Can you please check if it's working fine as required.
Also, it failed Travis build with the first commit. I can't figure out why.

@ragesoss
Copy link
Member

The build problem was unrelated (and now fixed on master)... it looks like this doesn't break any tests that weren't already broken. It's probably ready to go once it has a spec to exercise the pagination functionality.

@ragesoss
Copy link
Member

Closing this, since the phone app isn't under activity development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants