Defect #37585
closedDo not show "History" tab for content in Filesystem repository
0%
Description
Although Filesystem repository (#1393) does not support revisions, "History" tab is shown for content in a Filesystem repository.
Another tab, "Annotate", which is not available in a Filesystem repository, is hidden. "History" tab should also be hidden.
Files
Related issues
Updated by Mischa The Evil about 2 years ago
Go MAEDA wrote:
I agree. I think this can be solved by adding:[...] "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?
Updated by Go MAEDA about 2 years ago
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?
Updated by Mischa The Evil about 2 years ago
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).
Repository#supports_all_revisions?
is additionally used for:
- 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?
Updated by Go MAEDA about 2 years ago
- File 37585.patch 37585.patch added
- Target version set to Candidate for next minor release
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 toRepository#supports_history?
.
I agree, naming is important. I will commit the attached patch for 5.0.3. After that, I will open a new issue that suggests changing the name of the method from supports_all_revisions?
to supports_history?
.
Updated by Go MAEDA about 2 years ago
- Target version changed from Candidate for next minor release to 5.0.3
Setting the target version to 5.0.3.
Updated by Go MAEDA about 2 years ago
- Status changed from New to Resolved
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch.
Updated by Go MAEDA about 2 years ago
- Related to Patch #37657: Rename Repository#supports_all_revisions? to Repository#supports_history? added
Updated by Go MAEDA about 2 years ago
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 toRepository#supports_history?
.
I have opened #37657.