GitHub Classroom Roster Assignment Paging Error

[ccannon94]

When selecting "Unlinked GitHub accounts" tab on the Class Roster Management Page, changing pages of the list will send the user back to the "All students" tab. Then, when "Unlinked GitHub accounts" is selected again, it is on the correct page. Not impossible to use, just weird.

For example, when on page 1 of "Unlinked GitHub accounts", when I select page 2, the page reload brings me back to the "All students" tab. However, when I click the "Unlinked Student accounts" tab, I am on page 2 as requested.

Preferably, selecting a page in "Unlinked GitHub accounts" would just refresh on the desired page.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sethb157]

Hi! I want to start contributing to Classroom, and I figured this might be a good place to start. I've been looking at the code--seems like the JavaScript this is relying on is acting a bit interesting across pages. Was wondering if there's someone I can send a few questions to. @mozzadrella, could you point me to the right person? Thanks!

[Kadaaran]

Hi @sethb157 👋 Awesome ! Feel free to ask me any questions and I'll try so answer to all of them as precisely as I can 💪

[sethb157]

Ok awesome! There are two places where the All Students/Unlinked Students tabs show up: once on the Assignment Detail view and once in the Roster Management view. The way things work now, the two are connected. That is, the following happens: 1) Start on Assignment Detail view, with All Students tab open 2) Go to Roster Management view, click on Unlinked Students tab 3) Go back to Assignment Detail view, Unlinked Students tab is now open, because you clicked on it in Roster Management.

I’m assuming this isn’t expected behavior??😬 if it is okay/expected, then this bug is a one-line fix. If not, It’ll be a little more refactoring to make the two act independently. Thoughts?

[sethb157]

If you need more clarification, please let me know!

[Kadaaran]

@sethb157 Oops. The expected behaviour is that the two paginations should work independently from each other.

What happens is that in the view, the Javascript we load is the roster one https://github.com/education/classroom/blob/4346984e6ca87e555329777b3df0e9ff926ab015/app/views/assignments/show.html.erb#L96

That's why I think the targeted tabs are the same no matter page you change. One solution could be to set re-init the variables every time we load or something like that 🤔 Create a new file would add duplicated code and that might not be ideal.

[sethb157]

@Kadaaran Ok, great. I didn't think that was expected behavior but just wanted to make sure. I'll get working on this. One last thing, what type of tests would you expect for this issue? I'm thinking a feature spec is probably best??

[Kadaaran]

@sethb157 Yeah that would be great but we don't have Capybara for now 😞 We are going to update the app to Rails 5.2 (integration tests with Capybara are built in 🎉)

In the meantime, you can start to work on it, open a PR (even if it's WIP )and then as soon as we update to 5.2 then you could start the test with Capybara.

Would that work on your end ?

[sethb157]

@Kadaaran that works for me!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[ccannon94]

Please don't close this again 😅

[BenEmdon]

@ccannon94 stalebot no longer can close issues! I have my :eyes: on you @stalebot

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.