Redmine

Go MAEDA wrote:

[...] "History" tab should also be hidden.

• a Repository#supports_history? method that would return 'true';
• a Repository::Filesystem#supports_history? method that would return 'false';
• a condition (if @repository.supports_history?) in source:/app/views/repositories/_link_to_functions.html.erb@21772#L11 for the rendering of the history tab.

What do you think?

Mischa The Evil wrote:

Go MAEDA wrote:
I agree. I think this can be solved by adding:

• a Repository#supports_history? method that would return 'true';
• a Repository::Filesystem#supports_history? method that would return 'false';
• a condition (if @repository.supports_history?) in source:/app/views/repositories/_link_to_functions.html.erb@21772#L11 for the rendering of the history tab.

Can we use the existing Repository#supports_all_revisions? instead of adding a new method?

Go MAEDA wrote:

Can we use the existing Repository#supports_all_revisions? instead of adding a new method?

I've taken a closer look at Repository#supports_all_revisions? and I think we can indeed. However, I think that the name of that method combined with what it's used for, can be quite confusing (which is also what led me to my previous proposal). I'll elaborate.

Repository#supports_all_revisions? was introduced in r5143 in support of r5145, as such was its original purpose to use it to make a distinction for rendering either the "view all revisions" or the "view revisions" link.
Following its introduction Etienne opened #7984, for which several changes were applied. And, while that issue remains open until today, I think it is actually solved with the commits that are related to the issue, but left open because of the additional (IMHO out-of-scope) matters that came up with it (see #7984#note-22 and #7984#note-23).

• the statistics link condition;
• the revision text field tag condition;
• the atom links condition.

Given the above, we now already have four uses for the method and with the proposal for this issue, it'll make five.
The problem with this isn't the usage itself. Instead, I think it's the name of the method which can be confusing in itself and as such be problematic.

TL;DR: I think that Repository#supports_all_revisions? can be used as-is for a new condition to hide the history tab in a 5.0.x maintenance release.
For 5.1.x (or 6.x) though, I'd say it might be more accurate to rename the method to Repository#supports_history?.

What do you think?

Mischa The Evil wrote:

TL;DR: I think that Repository#supports_all_revisions? can be used as-is for a new condition to hide the history tab in a 5.0.x maintenance release.
For 5.1.x (or 6.x) though, I'd say it might be more accurate to rename the method to Repository#supports_history?.

I have opened #37657.