FIX: Correct version comparison logic when comparing stable to beta (#10135)

FIX: Correct version comparison logic when comparing stable to beta (#10135)

  • FIX: Correct version comparison logic when comparing stable to beta

For example, version 1.3.0 should be considered higher than 1.3.0.beta3. So Discourse.has_needed_version?('1.3.0', '1.3.0.beta3') should return true

  • Switch to use Gem::Version to compare versions
diff --git a/lib/version.rb b/lib/version.rb
index ab83043..dbb5a6b 100644
--- a/lib/version.rb
+++ b/lib/version.rb
@@ -16,22 +16,6 @@ module Discourse
   end
 
   def self.has_needed_version?(current, needed)
-    current_split = current.split('.')
-    needed_split = needed.split('.')
-
-    (0..[current_split.size, needed_split.size].max).each do |idx|
-      current_str = current_split[idx] || ''
-
-      c0 = (needed_split[idx] || '').sub('beta', '').to_i
-      c1 = (current_str || '').sub('beta', '').to_i
-
-      # beta is less than stable
-      return false if current_str.include?('beta') && (c0 == 0) && (c1 > 0)
-
-      return true if c1 > c0
-      return false if c0 > c1
-    end
-
-    true
+    Gem::Version.new(current) >= Gem::Version.new(needed)
   end
 end
diff --git a/spec/components/version_spec.rb b/spec/components/version_spec.rb
index 5017983..255ef64 100644
--- a/spec/components/version_spec.rb
+++ b/spec/components/version_spec.rb
@@ -30,12 +30,20 @@ describe Discourse::VERSION do
       expect(Discourse.has_needed_version?('1.12.0', '2.12.5')).to eq(false)
     end
 
-    it "works for beta comparisons" do
+    it "works for beta comparisons when current_version is beta" do
       expect(Discourse.has_needed_version?('1.3.0.beta3', '1.2.9')).to eq(true)
       expect(Discourse.has_needed_version?('1.3.0.beta3', '1.3.0.beta1')).to eq(true)
       expect(Discourse.has_needed_version?('1.3.0.beta3', '1.3.0.beta4')).to eq(false)
       expect(Discourse.has_needed_version?('1.3.0.beta3', '1.3.0')).to eq(false)
     end
 
+    it "works for beta comparisons when needed_version is beta" do
+      expect(Discourse.has_needed_version?('1.2.0', '1.3.0.beta3')).to eq(false)
+      expect(Discourse.has_needed_version?('1.2.9', '1.3.0.beta3')).to eq(false)
+      expect(Discourse.has_needed_version?('1.3.0.beta1', '1.3.0.beta3')).to eq(false)
+      expect(Discourse.has_needed_version?('1.3.0.beta4', '1.3.0.beta3')).to eq(true)
+      expect(Discourse.has_needed_version?('1.3.0', '1.3.0.beta3')).to eq(true)
+    end
+
   end
 end

GitHub sha: 0edffcc4

This commit appears in #10135 which was approved by CvX. It was merged by SamSaffron.