Fix auto upgrade logic (#758)

app/helpers/upgrades_helper.rb

@@ -1,12 +1,12 @@
 module UpgradesHelper
-  def upgrade_notification
+  def get_upgrade_for_notification(user, upgrades_mode)
     return nil unless ENV["UPGRADES_ENABLED"] == "true"
 
     completed_upgrade = Upgrader.completed_upgrade
-    return completed_upgrade if completed_upgrade && Current.user.last_alerted_upgrade_commit_sha != completed_upgrade.commit_sha
+    return completed_upgrade if completed_upgrade && user.last_alerted_upgrade_commit_sha != completed_upgrade.commit_sha
 
     available_upgrade = Upgrader.available_upgrade
-    if available_upgrade && Setting.upgrades_mode == "manual" && Current.user.last_prompted_upgrade_commit_sha != available_upgrade.commit_sha
+    if available_upgrade && upgrades_mode == "manual" && user.last_prompted_upgrade_commit_sha != available_upgrade.commit_sha
       available_upgrade
     end
   end

app/models/upgrader/upgrade.rb

@@ -13,7 +13,9 @@ class Upgrader::Upgrade
   end
 
   def available?
-    version > Maybe.version || (version == Maybe.version && commit_sha != Maybe.commit_sha)
+    return false if commit_sha == Maybe.commit_sha || version < Maybe.version
+    return false if version == Maybe.version && type == "release"
+    true
   end
 
   def to_s

app/views/layouts/with_sidebar.html.erb

@@ -12,7 +12,9 @@
     </main>
   </div>
 
-  <%= render "shared/upgrade_notification" %>
+  <% if (upgrade = get_upgrade_for_notification(Current.user, Setting.upgrades_mode)) %>
+    <%= render partial: "shared/upgrade_notification", locals: { upgrade: upgrade } %>
+  <% end %>
 <% end %>
 
 <%= render template: "layouts/application" %>

app/views/shared/_upgrade_notification.html.erb

@@ -1,19 +1,17 @@
-<% if upgrade_notification %>
+<%# locals: (upgrade:) %>
-  <% upgrade = upgrade_notification %>
+<div class="bg-white space-y-4 text-right fixed bottom-10 right-10 p-5 border border-alpha-black-200 shadow-xs rounded-md z-50 max-w-[350px]">
-  <div class="bg-white space-y-4 text-right fixed bottom-10 right-10 p-5 border border-alpha-black-200 shadow-xs rounded-md z-50 max-w-[350px]">
+  <div>
-    <div>
+    <p><%= link_to upgrade.to_s, upgrade.url, class: "text-sm text-blue-500 underline hover:text-blue-700", target: "_blank" %></p>
-      <p><%= link_to upgrade.to_s, upgrade.url, class: "text-sm text-blue-500 underline hover:text-blue-700", target: "_blank" %></p>
+    <% if upgrade.complete? %>
-      <% if upgrade.complete? %>
+      <p class="text-gray-900"><%= t(".app_upgraded", version: upgrade.to_s) %></p>
-        <p class="text-gray-900"><%= t(".app_upgraded", version: upgrade.to_s) %></p>
+    <% else %>
-      <% else %>
+      <p class="text-gray-900"><%= t(".new_version_available") %></p>
-        <p class="text-gray-900"><%= t(".new_version_available") %></p>
+    <% end %>
-      <% end %>
-    </div>
-    <div class="flex justify-end items-center gap-2">
-      <%= button_to t(".dismiss"), acknowledge_upgrade_path(upgrade.commit_sha), method: :post, class: "#{upgrade.complete? ? 'bg-gray-900 text-white' : 'bg-gray-100 text-gray-900'} text-sm font-bold p-2 rounded-lg" %>
-      <% if upgrade.available? %>
-        <%= button_to t(".upgrade_now"), deploy_upgrade_path(upgrade.commit_sha), method: :post, class: "bg-gray-900 hover:bg-gray-700 text-white font-medium text-sm p-2 rounded-lg" %>
-      <% end %>
-    </div>
   </div>
-<% end %>
+  <div class="flex justify-end items-center gap-2">
+    <%= button_to t(".dismiss"), acknowledge_upgrade_path(upgrade.commit_sha), method: :post, class: "#{upgrade.complete? ? 'bg-gray-900 text-white' : 'bg-gray-100 text-gray-900'} text-sm font-bold p-2 rounded-lg" %>
+    <% if upgrade.available? %>
+      <%= button_to t(".upgrade_now"), deploy_upgrade_path(upgrade.commit_sha), method: :post, class: "bg-gray-900 hover:bg-gray-700 text-white font-medium text-sm p-2 rounded-lg" %>
+    <% end %>
+  </div>
+</div>

test/models/upgrader/upgrade_test.rb

@@ -0,0 +1,36 @@
+require "test_helper"
+
+class UpgradeTest < ActiveSupport::TestCase
+  setup do
+    data = {
+      commit_sha: "latestcommit",
+      version: Semver.new("0.1.0-alpha.2")
+    }
+
+    @commit_upgrade = Upgrader::Upgrade.new "commit", data
+    @release_upgrade = Upgrader::Upgrade.new "release", data
+  end
+
+  test "available if latest commit and app not upgraded" do
+    Maybe.stubs(:version).returns(@commit_upgrade.version)
+    Maybe.stubs(:commit_sha).returns("outdatedcommitsha")
+
+    assert @commit_upgrade.available?
+    assert_not @release_upgrade.available?
+  end
+
+  test "available if latest release and app not upgraded" do
+    Maybe.stubs(:version).returns(Semver.new("0.1.0-alpha.1"))
+    Maybe.stubs(:commit_sha).returns("outdatedcommitsha")
+
+    assert @commit_upgrade.available?
+    assert @release_upgrade.available?
+  end
+
+  test "not available if app commit greater or equal to" do
+    Maybe.stubs(:version).returns(@commit_upgrade.version)
+    Maybe.stubs(:commit_sha).returns(@commit_upgrade.commit_sha)
+
+    assert_not @commit_upgrade.available?
+  end
+end