Project

General

Profile

Actions

Defect #37585

closed

Do not show "History" tab for content in Filesystem repository

Added by Go MAEDA about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
SCM
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

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

history-tab.png (79.7 KB) history-tab.png Go MAEDA, 2022-08-19 03:14
37585.patch (1.95 KB) 37585.patch Go MAEDA, 2022-09-03 17:01

Related issues

Related to Redmine - Patch #37657: Rename Repository#supports_all_revisions? to Repository#supports_history?ClosedGo MAEDA

Actions
Actions #1

Updated by Mischa The Evil about 2 years ago

Go MAEDA wrote:

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

I agree. I think this can be solved by adding:

What do you think?

Actions #2

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:

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

Actions #3

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).

Nowadays 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?

Actions #4

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 to Repository#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?.

Actions #5

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.

Actions #6

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.

Actions #7

Updated by Go MAEDA about 2 years ago

  • Related to Patch #37657: Rename Repository#supports_all_revisions? to Repository#supports_history? added
Actions #8

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 to Repository#supports_history?.

I have opened #37657.

Actions #9

Updated by Go MAEDA about 2 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF