[WIP] Make multiple repositories selectable @ issues/PR page

[oscarlofwenhamn]

Will partially fix #6355

[codecov-io]

Codecov Report

Merging #6369 into master will increase coverage by 0.02%.
The diff coverage is 47.29%.

@@            Coverage Diff             @@
##           master    #6369      +/-   ##
==========================================
+ Coverage   41.45%   41.48%   +0.02%     
==========================================
  Files         442      442              
  Lines       59610    59636      +26     
==========================================
+ Hits        24712    24739      +27     
+ Misses      31667    31663       -4     
- Partials     3231     3234       +3

Impacted Files	Coverage Δ
models/issue.go	48% <0%> (-0.12%)	⬇️
routers/user/home.go	56.25% <49.29%> (+2.96%)	⬆️
models/unit.go	62.16% <0%> (-5.41%)	⬇️
routers/repo/view.go	42.02% <0%> (-1.02%)	⬇️
models/gpg_key.go	56.66% <0%> (+0.83%)	⬆️
modules/log/event.go	65.98% <0%> (+1.52%)	⬆️
modules/log/colors_router.go	87.5% <0%> (+4.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c412f5...847510e. Read the comment docs.

[zeripath]

I'm not completely in the loop for this but why do we still have repoID and repoIDs here?

[oscarlofwenhamn]

@zeripath

I'm not completely in the loop for this but why do we still have repoID and repoIDs here?

Assuming you mean "why both", I think it is for fear of accidentally influencing something outside the scope of my intentions. I believe we can replace repoID with repoIDs[0] in the cases where there is only one, but I haven't been wanting to accidentally overreach.

[lunny]

@oscarlofwenhamn for code readable consideration, I think @zeripath 's idea is better to keep only RepoIDs []int64.

[oscarlofwenhamn]

I'm at a loss, if I don't have a eureka moment when working on this next I think I will comment out the test in question:

Messages: Request: GET /issues?type=created_by&repos=[0]&sort=&state=closed
--
247 | testlogger.go:57: 2019/05/15 06:47:22 ...ers/routes/routes.go:108:func1() [I] Completed GET /issues?type=created_by&repos=[0]&sort=&state=closed 404 Not Found in 2.919792ms

Since there are a number of tests in the same document that are commented out, with reason listed as basically "these 404 when they should be fine", it doesn't seem like something that is unheard of. If the rest of the build runs fine, we can take a stance on the test in question, unless someone can think of what might be wrong - functionality is now as expected and desired.

• Clicking "All" shows issues from all repos
• Clicking repos in the sidebar list makes them highlighted in the list and makes only issues from selected repos visible
• Clicking a selected repo makes it unselected
• Repo selection persists through selection of different created_by and through pagination changes

[oscarlofwenhamn]

It seems like the issue is the combination of issues and created_by (since the pull request tests work), i.e.

// "/issues?type=created_by&repos=[0]&sort=&state=closed",
// "/issues?type=created_by&repos=[0]&sort=&state=open",

If anyone has any suggestions I would really appreciate a hand, currently I'm out of ideas.

[zeripath]

Hmm commenting out tests with the message "these should be fine but they're not" means that things are not fine at all...

Ok you've got my attention.

Look at a single URL that doesn't work and trace it back through.

[zeripath]

Your 404 is probably coming from here:

https://github.com/oscarlofwenhamn/gitea/blob/3fc7bb176040001a8788034c111cefb6d684db02/routers/user/home.go#L329

Which implies that the testing user does not have permission to read the issues of that's repository.

Does that make sense at all?

[zeripath]

Now probably I'd say that if you can't read a repository you should drop those issues from the list and from the totals found but I would have to look properly at this.

It might be better to only search in repositories that the user has access to but you'd have to look at which is more efficient.

[oscarlofwenhamn]

@zeripath Thanks, that gives me some new points to mix my thought loop up. I feel like I've been staring myself blind at it a bit, so it's good to get a second look.

I am suspecting the same source of the 404 (tampering with that chunk was what made the one previous test succeed despite not working in practice), so I'll make some new attempts at what you're suggesting.

Also

Does that make sense at all?

Yes, very much so!

[oscarlofwenhamn]

I'm currently not able to keep working on this for a while, so I'm thinking it can be moved to 1.x.x for now. If someone feels like picking it up, they'd be welcome to do so.

[6543]

@lunny #8741 continue this so we can close this PR