Defect #37585

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

Added by Go MAEDA about 1 month ago. Updated 24 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:SCM
Target version:5.0.3
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.

history-tab.png (79.7 KB) Go MAEDA, 2022-08-19 03:14

37585.patch Magnifier (1.95 KB) Go MAEDA, 2022-09-03 17:01


Related issues

Related to Redmine - Patch #37657: Rename Repository#supports_all_revisions? to Repository#s... Closed

Associated revisions

Revision 21800
Added by Go MAEDA 26 days ago

Don't show "History" tab for content in Filesystem repository (#37585).

Patch by Go MAEDA.

Revision 21802
Added by Go MAEDA 24 days ago

Merged r21800 from trunk to 5.0-stable (#37585).

History

#1 Updated by Mischa The Evil about 1 month ago

Go MAEDA wrote:

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

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

What do you think?

#2 Updated by Go MAEDA about 1 month 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?

#3 Updated by Mischa The Evil about 1 month 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?

#4 Updated by Go MAEDA 30 days ago

  • File 37585.patchMagnifier 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 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?.

#5 Updated by Go MAEDA 28 days ago

  • Target version changed from Candidate for next minor release to 5.0.3

Setting the target version to 5.0.3.

#6 Updated by Go MAEDA 26 days ago

  • Status changed from New to Resolved
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch.

#7 Updated by Go MAEDA 24 days ago

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

#8 Updated by Go MAEDA 24 days 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.

#9 Updated by Go MAEDA 24 days ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF