search

rubyforgood / pet-rescue

Pet Rescue is an application making it easy to link adopters/fosters with pets. We work with grassroots pet rescue organizations to understand how we can make the most impact.
MIT License
60 stars 103 forks source link

926 Staff Dashboard - make overdue tasks the default shown table #943

Closed Aaryanpal closed 2 weeks ago

Aaryanpal commented 2 weeks ago

🔗 Issue

926

✍️ Description

📷 Screenshots/Demos

Screencast from 2024-08-30 20-01-55.webm

kasugaijin commented 2 weeks ago

@Aaryanpal just need an update to the test - see broken test in pipeline - assuming you changes some text it expects to see.

kasugaijin commented 2 weeks ago

@Aaryanpal looks good! Just a few comments, nothing major. I notice we are using Pagy and expect 5 records, but in your demo video the paginated collections are much larger than 5. IT would be good if we keep this table small, at 5 records per page. Do you know why there is the discrepancy?

Aaryanpal commented 2 weeks ago

Screencast from 2024-08-30 23-42-49.webm

Aaryanpal commented 2 weeks ago

@Aaryanpal looks good! Just a few comments, nothing major. I notice we are using Pagy and expect 5 records, but in your demo video the paginated collections are much larger than 5. IT would be good if we keep this table small, at 5 records per page. Do you know why there is the discrepancy?

Wrong

  @pagy, paginated_adoptable_pets = pagy(
      @q.result,
      items: 9
    )

Right

  @pagy, paginated_adoptable_pets = pagy(
      @q.result,
      limit: 9
    )

We're using items to restrict the records, but we should be using limit instead to properly limit them. Could you create an issue so I can replace this throughout the entire codebase?

Reference

kasugaijin commented 2 weeks ago

@Aaryanpal looks good! Just a few comments, nothing major. I notice we are using Pagy and expect 5 records, but in your demo video the paginated collections are much larger than 5. IT would be good if we keep this table small, at 5 records per page. Do you know why there is the discrepancy?

Wrong

  @pagy, paginated_adoptable_pets = pagy(
      @q.result,
      items: 9
    )

Right

  @pagy, paginated_adoptable_pets = pagy(
      @q.result,
      limit: 9
    )

We're using items to restrict the records, but we should be using limit instead to properly limit them. Could you create an issue so I can replace this throughout the entire codebase?

Reference

Ah thank you, I made an issue here https://github.com/rubyforgood/pet-rescue/issues/944