ChiliProject is not maintained anymore. Please be advised that there will be no more updates.

We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.

Allow underscores in project identifiers (Feature #146)

Added by Derek Montgomery at 2011-02-04 04:58 pm. Updated at 2011-02-22 08:52 pm.

Status:Closed Start date:2011-02-04
Priority:Normal Due date:
Assignee:Felix Schäfer % Done:

0%
Category:-
Target version:1.1.0 — Bell
Remote issue URL: Affected version:

Description

If there is no specific reason to disallow underscores, can you please allow them to be used in project identifiers??
(Category Projects)

CP_underscore_in_identifier.patch (695 Bytes) Derek Montgomery, 2011-02-04 08:30 pm

Associated revisions

Revision 9ed2d8ed
Added by Felix Schäfer at 2011-02-22 09:50 pm

Allow underscores in project identifiers. #146

History

#1
Updated by Derek Montgomery at 2011-02-04 08:30 pm

Added patch file (thanks thegcat)

  • Target version set to 1.1.0 — Bell
  • File CP_underscore_in_identifier.patch added
  • Assignee set to Eric Davis
  • Status changed from Open to Ready for review

#2
Updated by Holger Just at 2011-02-04 08:50 pm

We use that patch in different production sites for some time now. No issues whatsoever. I vote go.

#3
Updated by Holger Just at 2011-02-04 08:52 pm

Oh, forgot, we still need a test for that. So I withdraw my vote until we have a test failing without the patch and passing with it.

  • Target version deleted (1.1.0 — Bell)
  • Assignee deleted (Eric Davis)
  • Status changed from Ready for review to Needs more information

#4
Updated by Eric Davis at 2011-02-05 01:29 am

Should we allow an identifier to start with an underscore also?

I also think there is a locale string that needs to be changed, identifier has a text hint:

  text_project_identifier_info: 'Only lower case letters (a-z), numbers and dashes are allowed.<br />Once saved, the identifier can not be changed.'

Holger: should we set the Status to Open? Needs more information is used if the request isn't clear.

#5
Updated by Holger Just at 2011-02-05 01:41 am

Eric Davis wrote:

Holger: should we set the Status to Open? Needs more information is used if the request isn't clear.

Probably better, yes.

  • Status changed from Needs more information to Open

#6
Updated by Felix Schäfer at 2011-02-05 11:02 am

Eric Davis wrote:

Should we allow an identifier to start with an underscore also?

I can't think of a reason not too.

#7
Updated by Felix Schäfer at 2011-02-20 05:25 pm

Holger Just wrote:

Oh, forgot, we still need a test for that. So I withdraw my vote until we have a test failing without the patch and passing with it.

I'll give it a try. Is this small enough for 1.2.0?

  • Assignee set to Felix Schäfer

#10
Updated by Felix Schäfer at 2011-02-21 08:45 pm

Proposed patch:

 1diff --git a/app/models/project.rb b/app/models/project.rb
 2index 8315a48..eb0da23 100644
 3--- a/app/models/project.rb
 4+++ b/app/models/project.rb
 5@@ -75,7 +75,7 @@ class Project < ActiveRecord::Base
 6   validates_length_of :homepage, :maximum => 255
 7   validates_length_of :identifier, :in => 1..IDENTIFIER_MAX_LENGTH
 8   # donwcase letters, digits, dashes but not digits only
 9-  validates_format_of :identifier, :with => /^(?!\d+$)[a-z0-9\-]*$/, :if => Proc.new { |p| p.identifier_changed? }
10+  validates_format_of :identifier, :with => /^(?!\d+$)[a-z0-9\-_]*$/, :if => Proc.new { |p| p.identifier_changed? }
11   # reserved words
12   validates_exclusion_of :identifier, :in => %w( new )
13
14diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb
15index 8eb79fd..936008b 100644
16--- a/test/unit/project_test.rb
17+++ b/test/unit/project_test.rb
18@@ -101,6 +101,7 @@ class ProjectTest < ActiveSupport::TestCase
19     to_test = {"abc" => true,
20                "ab12" => true,
21                "ab-12" => true,
22+               "ab_12" => true,
23                "12" => false,
24                "new" => false}
25

As this doesn't break backwards compatibility, I'd even put it in 1.1, feel free to reschedule if you don't agree.

  • Target version set to 1.1.0 — Bell
  • Status changed from Open to Ready for review

#11
Updated by Eric Davis at 2011-02-22 07:18 pm

Felix:

Need to change the i18n strings too. See note 4 above

#12
Updated by Felix Schäfer at 2011-02-22 08:52 pm

I didn't want to spam with the locale changes :-) Committed in 9ed2d8ed77906c21f6df8331008aad2adcf23148.

  • Status changed from Ready for review to Closed

Also available in: Atom PDF