Make "Ghost" not link to 404 page

[oscarlofwenhamn]

Fixes #6270

• Add check to see that user id >0 before adding clickable link to user name

[codecov-io]

Codecov Report

Merging #6410 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #6410      +/-   ##
=========================================
+ Coverage   39.39%   39.4%   +<.01%     
=========================================
  Files         393     393              
  Lines       53264   53271       +7     
=========================================
+ Hits        20986   20989       +3     
- Misses      29292   29297       +5     
+ Partials     2986    2985       -1

Impacted Files	Coverage Δ
models/issue.go	47.99% <0%> (-0.11%)	⬇️
models/repo_list.go	66.84% <0%> (-1.06%)	⬇️
modules/process/manager.go	81.15% <0%> (+4.34%)	⬆️

---

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 ee0d3ee...b01e186. Read the comment docs.

[axifive]

@oscarlofwenhamn, I also found the same link here: gitea/templates/repo/issue/milestone_issues.tmpl

[zeripath]

Hmm... Shouldn't we just have these users link to a fake page saying that they're no longer real users. Then you don't need to have these multiple "fake" variants?

[oscarlofwenhamn]

@axifive

I also found the same link here: gitea/templates/repo/issue/milestone_issues.tmpl

Thanks, I'll sort them out probably tomorrow.
Fixed in 8c67cab.

@zeripath Personally, I disagree, as I think making a link at that point leads to unnecessary jumping about on the site, which I think I myself would be frustrated with. I would rather add a hover text instead, with an explanation.

[zeripath]

It looks like you're missing one more label - issue_opened_by_fake

My only worry is that we're creating multiple copies of the same code and template.

Perhaps, it's better to use a subtemplate? I dunno, but it's certainly distasteful to have these multiple copies of the same code.

I think it's also worth looking at what the API gives. I suspect 404 is probably acceptable as a response to looking at a non existent user, but I wonder, is it always clear in our responses that the user doesn't exist?

[oscarlofwenhamn]

It looks like you're missing one more label - issue_opened_by_fake

Thanks, I'll sort it to make the PR more complete, regardless of what we'll do with it.

Perhaps, it's better to use a subtemplate?

That's not a bad idea at all, adding a template would mean it's all about adding a one-liner to the relevant places. I'll look into it, unless the consensus is to keep the links clickable and rather review the 404 responses. Again, my vote is to display the "This user does not exist"-info on hover or something like that, and make Ghost non-clickable.

[zeripath]

I think you're right, making the ghost users non clickable is most right. (We probably do need a nonexistent user page in case any are missed or someone tries to craft a link to one.)

[oscarlofwenhamn]

Agreed: Say I type out an explicit link to a collaborators page in a README, and later that user is removed, we'd want to have something catching that. Though, I think that is outside the scope of this PR. Either way, I'll continue with the last label and look at making a subtemplate.

[oscarlofwenhamn]

@zeripath I've ran into an issue: I need to either pass the variable $timeStr to the new template or, if I pass the current element of .Issues and evaluate the $timeStr in the new template, I need a way to pass $.i18n into the template, as currently I can't figure out how to do both. Do you have any suggestion?

Something like

{{ range .Issues }}
...
{{ $timeStr := TimeSinceUnix .GetLastEventTimestamp $.Lang }}
{{ template "repo/issue/poster_link" {{. , $timeStr}} }}
...

so I can evaluate the time outside the template would be preferred, in my opinion, if possible.

[zeripath]

I think any template to create the user link should just be as simple as possible - almost be as simple as calling a function on the user, eg. user.Anchor().

If it's not clear how to do it - we can just do the fakes and think about it later.