fix: pagination not working correctly

[kleinfreund]

Fixes pagination not working correctly on some pages.

Adds the logic for persisting the pagination offset in the URL so reloading reloads the page the user was on.

Fixes #441.

Signed-off-by: Philipp Rudloff [email protected]

[netlify]

✅ Deploy Preview for kuma-gui ready!

Name	Link
🔨 Latest commit	705b2b0
🔍 Latest deploy log	https://app.netlify.com/sites/kuma-gui/deploys/6375ec11b2436700085a2988
😎 Deploy Preview	https://deploy-preview-460--kuma-gui.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[johncowen]

I had a go a repro-ing the bug here, am I right in thinking its when clicking on the [Next] pagination button a few times and then clicking [Previous] the [Previous] button immediately disappears? (plus of course the page/offset not being in the URL)

Couple of things I noticed that I wanted to ask about also:

• At first glance (without looking at the code) it looks like the value of next (and therefore the offset for next sent from the backend) in the API response isn't being used, did I follow right?
• It looks like every time I click next or previous it makes an additional request to the 'item' in the preview panel thing, is that something we want to prevent? I wasn't massively sure whether it was the request for the already displayed 'item' or the first item in the next page

[kleinfreund]

I had a go a repro-ing the bug here, am I right in thinking its when clicking on the [Next] pagination button a few times and then clicking [Previous] the [Previous] button immediately disappears? (plus of course the page/offset not being in the URL)

@johncowen Do you mean you’re using the current master branch version and this is what you found or do you mean you’re using this PR’s branch and that’s what’s wrong with it? If the latter, on which page are you trying this?

At first glance (without looking at the code) it looks like the value of next (and therefore the offset for next sent from the backend) in the API response isn't being used, did I follow right?

Yes. That is correct. We rely on the offsets used in the frontend to be correct which is brittle, but that’s how this was built. We could switch to using the URLs from the response.

It looks like every time I click next or previous it makes an additional request to the 'item' in the preview panel thing, is that something we want to prevent? I wasn't massively sure whether it was the request for the already displayed 'item' or the first item in the next page

This happens when selecting an item in the list either via user interaction or when loading a list (e.g. when navigating to the view, when switching the mesh, when changing the page). In this view we could avoid this additional request because the detail information of a policy is already present on the response retrieved from the collection resource endpoint. However that’s something we should consider outside the scope of this PR. It would require a bit of additional work to properly clone objects (i.e. deep copy) from the response to avoid writes to one object affecting that of its source (this recently caused a bug where data unintentionally leaked into the detail section of this view).

[johncowen]

@johncowen Do you mean you’re using the current master branch version and this is what you found or do you mean you’re using this PR’s branch and that’s what’s wrong with it? If the latter, on which page are you trying this?

Yeah, I'm using the master branch to try to understand what the fix is for first (so I then check this out to see the fix working after), I was trying the circuit-breaker listing but from a brief glance at the code it looks like all listings.

We rely on the offsets used in the frontend to be correct which is brittle, but that’s how this was built. We could switch to using the URLs from the response.

You are on exactly the same wave length as me! 😄 Yeah don't worry I'm trying to understand how it's built as I go, not knocking anything.

This happens when selecting an item in the list either via user interaction or when loading a list (e.g. when navigating to the view, when switching the mesh, when changing the page). In this view we could avoid this additional request because the detail information of a policy is already present on the response retrieved from the collection resource endpoint. However that’s something we should consider outside the scope of this PR. It would require a bit of additional work to properly clone objects (i.e. deep copy) from the response to avoid writes to one object affecting that of its source (this recently caused a bug where data unintentionally leaked into the detail section of this view).

Gotcha, yeah defo not something to address here, just trying to understand how things work currently again!

---

I've been having a little look at it now I understand what the fix is for here, all looks fine and dandy to me. Let me stare at the code a bit see what I can pick up there, it's good for some Vue learning also, be back soon (but probably first thing in the morning now)

[johncowen]

nice fix!