package/show sometimes shows wrong rev

[bmwiedemann]

Issue Description

package/show sometimes shows wrong (latest) revision I noticed that https://build.opensuse.org/package/show/X11:XOrg/intel-vaapi-driver?rev=5 as linked from https://build.opensuse.org/package/revisions/X11:XOrg/intel-vaapi-driver?page=2 shows the wrong rev. However, rev=1 works, so it is not always broken.

Expected Result

should show the old revision specified by the rev= parameter or print an error message why that is not possible.

How to Reproduce

1. Go to https://build.opensuse.org/package/show/X11:XOrg/intel-vaapi-driver?rev=5

[hennevogel]

Links expand by default. Are you looking for the unexpanded sources?

https://build.opensuse.org/package/show/X11:XOrg/intel-vaapi-driver?rev=5&expand=0

[bmwiedemann]

Indeed that looks better. So shouldn't the "Browse Source" link on https://build.opensuse.org/package/revisions/X11:XOrg/intel-vaapi-driver?page=2 have the &expand=0 then?

[hennevogel]

That's quite a philosophical debate. The answer so far, no.

[bmwiedemann]

I'd argue, the current behavior violates the principle of least surprise.

When the UI provides all these links on the revisions page, my expectation as a user is that it lets me view the old revisions as they were back then at the date specified. Similar to osc up -r $rev that let me access those as expected (I used that a workaround when I did not know that the expand=0 would help).

[hennevogel]

osc ls -R 5 X11:XOrg intel-vaapi-driver on the other hand....

[bmwiedemann]

osc ls -R 5 X11:XOrg intel-vaapi-driver on the other hand....

That is actually showing both layers, and has a hint # -> openSUSE:Factory intel-vaapi-driver (latest) which is useful to avoid surprise.

I wonder why osc ls -v -R 5 X11:XOrg intel-vaapi-driver behaves differently from that (just shows local unexpanded version).

[bmwiedemann]

If you think, it should stay on expand=1 - how about adding that part explicitly to the URL to make it easier to see and change? Though experience tells me that your web-application needs some improvement if editing URLs is the easiest way to do what you want.

... and that finally made me notice that the usual "(show unmerged sources)" link is missing when there is a rev= param. Why?

[hennevogel]

... and that finally made me notice that the usual "(show unmerged sources)" link is missing when there is a rev= param. Why?

That is a very good question I tried to understand yesterday, but didn't get to a conclusion. Links are complicated...

[bmwiedemann]

Seems to originate from commit 0850a3b06024dfd2683cedb1966569864989e3ef by @coolo

[webui] Fix links between files

don't suggest to switch between expand=0 and expand=1 if a revision
argument exists

[bmwiedemann]

In that context of <% if @expand && @expand.to_s == "1" %> it was introduced when expand=0 was the implicit default. So it was meant to always show the expand=0 version to users when rev= was specified.

I guess, at some point, the implicit expand default was changed, but the logic for when to show links was not adapted along with it.

[dmarcoux]

As mentioned in #12689, it seems like we don't want to have the changes requested in this issue. I'm closing this issue. Feel free to re-open if I was mistaken.

[bmwiedemann]

I think, the issue is still valid - but might need a different solution. It could be as easy as to show the link to unexpanded sources again as a quasi-revert of @coolo 's commit 0850a3b06024dfd2683cedb1966569864989e3ef which was meant to keep people on expand=0, before expand=1 became the default.

Can you please re-open?

[bmwiedemann]

I'd still like to get this issue fixed. To help with that, let me summarize:

• the /package/revisions page links to a /package/show...?rev=N page without mentioning expand= and gets expanded to show the wrong content
• commit 0850a3b06024dfd2683cedb1966569864989e3ef hides the toggle to alter the expand bit, because the view was meant to only be used by the then-default expand=0 - but today we seem to default to expand=1, though some sources still think otherwise.
• I could not yet locate the commit that changed the default to expand=1 (hints welcome) - however it seems, we pass the value through to the backend ; and https://github.com/openSUSE/open-build-service/commit/31533ee608e015225f5375c0e055d1c7c14ba10c even suggests that expand=1 was the default even before coolo's 2012 commit

So was expand ever defaulting to 0? I think, we should fix show.html.haml to also assume a default expand=1 - or do I miss something?