Skip to content

Commit

Permalink
Allow deployments to be soft-deleted (#565)
Browse files Browse the repository at this point in the history
So that we can edit people's deployments without losing history for historical releases, at least from the backend for the time being.

Eventually in the future, we should have model snapshots.
  • Loading branch information
kitallis authored Oct 13, 2023
1 parent 8579c1a commit 61052b6
Show file tree
Hide file tree
Showing 15 changed files with 104 additions and 14 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ gem "google-apis-firebase_v1beta1", "~> 0.35.0"
gem "initials", "~> 0.4.3"
gem "rack-attack", "~> 6.7"
gem "june-analytics-ruby", "~> 2.4"
gem "discard", "~> 1.3"

group :development, :test do
gem "debug", platforms: %i[mri mingw x64_mingw]
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ GEM
diff-lcs (1.5.0)
digest-crc (0.6.4)
rake (>= 12.0.0, < 14.0.0)
discard (1.3.0)
activerecord (>= 4.2, < 8)
docile (1.4.0)
dogstatsd-ruby (5.5.0)
domain_name (0.5.20190701)
Expand Down Expand Up @@ -612,6 +614,7 @@ DEPENDENCIES
debug
device_detector (~> 1.0)
devise (~> 4.8)
discard (~> 1.3)
dogstatsd-ruby (~> 5.5)
dotenv-rails (~> 2.7)
down (~> 5.3)
Expand Down
2 changes: 1 addition & 1 deletion app/libs/queries/builds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def count

def android_all
selected_android_records.to_a.map do |record|
deployments = record.step_run.step.deployments
deployments = record.step_run.deployment_runs.map(&:deployment)
attributes =
record
.attributes
Expand Down
4 changes: 3 additions & 1 deletion app/models/deployment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# id :uuid not null, primary key
# build_artifact_channel :jsonb indexed => [integration_id, step_id]
# deployment_number :integer default(0), not null, indexed => [step_id]
# discarded_at :datetime indexed
# is_staged_rollout :boolean default(FALSE)
# staged_rollout_config :decimal(, ) default([]), is an Array
# created_at :datetime not null
Expand All @@ -15,6 +16,7 @@
class Deployment < ApplicationRecord
has_paper_trail
include Displayable
include Discard::Model

self.implicit_order_column = :deployment_number

Expand Down Expand Up @@ -46,7 +48,7 @@ class Deployment < ApplicationRecord
def staged_rollout? = is_staged_rollout

def set_deployment_number
self.deployment_number = step.deployments.maximum(:deployment_number).to_i + 1
self.deployment_number = step.all_deployments.maximum(:deployment_number).to_i + 1
end

def external?
Expand Down
4 changes: 4 additions & 0 deletions app/models/release.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ def stoppable?
may_stop?
end

def end_time
completed_at || stopped_at
end

def fetch_commit_log
if upcoming?
ongoing_head = train.ongoing_release.first_commit
Expand Down
13 changes: 11 additions & 2 deletions app/models/step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ class Step < ApplicationRecord

belongs_to :release_platform, inverse_of: :steps
has_many :step_runs, inverse_of: :step, dependent: :destroy
has_many :deployments, -> { sequential }, inverse_of: :step, dependent: :destroy
has_many :deployments, -> { kept.sequential }, inverse_of: :step, dependent: :destroy
has_many :all_deployments, -> { sequential }, class_name: "Deployment", inverse_of: :step, dependent: :destroy
has_many :deployment_runs, through: :deployments

validates :ci_cd_channel, presence: true, uniqueness: {scope: :release_platform_id, message: "you have already used this in another step of this train!"}
validates :release_suffix, format: {with: /\A[a-zA-Z\-_]+\z/, message: "only allows letters and underscore"}, if: -> { release_suffix.present? }
validates :deployments, presence: true, on: :create
Expand Down Expand Up @@ -55,6 +55,15 @@ class Step < ApplicationRecord
delegate :android?, to: :app
delegate :ci_cd_provider, :notify!, to: :train

def active_deployments_for(release)
return deployments unless release
return deployments.where("created_at < ?", release.scheduled_at) if release.end_time.blank?

all_deployments
.where("created_at < ?", release.scheduled_at)
.where("discarded_at IS NULL OR discarded_at >= ?", release.end_time)
end

def set_step_number
self.step_number = release_platform.steps.review.maximum(:step_number).to_i + 1
release_platform.release_step&.update!(step_number: step_number.succ) if review?
Expand Down
2 changes: 1 addition & 1 deletion app/models/step_run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def status_summary
end

def first_deployment
step.deployments.find_by(deployment_number: 1)
step.deployments.order(deployment_number: :asc).first
end

def finished_deployments?
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/_deployments.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<ol>
<% step.deployments.each do |deployment| %>
<% step.active_deployments_for(platform_run&.release).each do |deployment| %>
<li class="last:mb-0 mb-1">
<div>
<%= render partial: "shared/deployment", locals: { deployment: deployment } %>

<% if show_deployment_status %>
<%= render partial: "shared/live_release/deployment_status",
locals: { step_run: step_run, deployment: deployment, release: release } %>
locals: { step_run: step_run, deployment: deployment, platform_run: platform_run } %>
<% else %>
<% if deployment.staged_rollout? %>
<div class="my-4 pt-4 ml-6 border-t border-dashed w-1/2">
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/_per_step_metadata.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<%= render partial: "trains/edit_step_button", locals: { editable: editable, step: step } %>
<% end %>

<% if release.present? %>
<% if platform_run.present? %>
<%= render partial: "shared/live_release/step_run_status", locals: { step_run: step_run, step: step } %>
<% end %>
</div>
Expand Down Expand Up @@ -40,7 +40,7 @@
<div class="flex flex-row items-center w-full">
<div class="inline-flex mr-1"></div>
<%= render partial: "shared/deployments",
locals: { step: step, step_run: step_run, release: release, show_deployment_status: release.present? } %>
locals: { step: step, step_run: step_run, platform_run:, show_deployment_status: platform_run.present? } %>
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/_step_tree_viz.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<ol class="mt-2">
<!-- review steps -->
<% release_platform.steps.review.includes(deployments: [:integration]).order(:step_number).each do |step| %>
<li><%= render partial: "shared/per_step_metadata", locals: { editable: editable, step: step, step_run: nil, release: nil } %></li>
<li><%= render partial: "shared/per_step_metadata", locals: { editable: editable, step: step, step_run: nil, platform_run: nil } %></li>
<%= render partial: "shared/step_tree_connector", locals: { color: step_color(step.kind) } %>
<% end %>

Expand All @@ -12,7 +12,7 @@
<% release_step = release_platform.steps.release.first %>
<% if release_step %>
<%= render partial: "shared/step_tree_connector", locals: { color: step_color("review") } %>
<li><%= render partial: "shared/per_step_metadata", locals: { editable: editable, step: release_step, step_run: nil, release: nil } %></li>
<li><%= render partial: "shared/per_step_metadata", locals: { editable: editable, step: release_step, step_run: nil, platform_run: nil } %></li>
<% end %>

<!-- add release button -->
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/live_release/_deployment_status.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<% if step_run.manually_startable_deployment?(deployment) %>
<%= authz_button_to :blue,
"Start this deployment",
start_release_step_run_deployment_path(release, step_run, deployment),
start_release_step_run_deployment_path(platform_run, step_run, deployment),
{ class: 'mt-2 btn-xs' } %>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/live_release/_stability.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@

<div>
<%= render partial: "shared/per_step_metadata",
locals: { editable: false, release: release, step: step, step_run: step_run } %>
locals: { editable: false, platform_run: release, step: step, step_run: step_run } %>
</div>
</li>

Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20231011072223_add_deleted_at_to_deployments.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddDeletedAtToDeployments < ActiveRecord::Migration[7.0]
def change
safety_assured do
add_column :deployments, :discarded_at, :datetime
add_index :deployments, :discarded_at
end
end
end
4 changes: 3 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 61 additions & 0 deletions spec/models/step_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,65 @@
expect(step.errors).to be_empty
end
end

describe "#active_deployments_for" do
let(:release_platform) { create(:release_platform) }
let(:train) { release_platform.train }

it "returns all non-discarded deployments when no release is passed in" do
step = create(:step, :with_deployment, release_platform: release_platform)
next_deployment = create(:deployment, step:)
_discarded_deployment = create(:deployment, discarded_at: Time.current, step:)

step.reload
expect(step.active_deployments_for(nil)).to contain_exactly(step.deployments.first, next_deployment)
end

it "returns deployments that were available in the past releases" do
two_days_ago = 2.days.ago
two_hours_ago = 2.hours.ago
four_hours_ago = 2.hours.ago
step = travel_to(two_days_ago) { create(:step, :with_deployment, release_platform: release_platform) }
d1 = create(:deployment, step:, created_at: two_days_ago)
d2 = create(:deployment, step:, created_at: two_days_ago)
old_release = create(:release, train:, scheduled_at: four_hours_ago, completed_at: two_hours_ago)

d2.discard!

step.reload
expect(step.active_deployments_for(old_release)).to contain_exactly(step.deployments.first, d1, d2)
end

it "returns deployments active at the duration of the release" do
two_days_ago = 2.days.ago
step = travel_to(two_days_ago) { create(:step, :with_deployment, release_platform: release_platform) }
d1 = create(:deployment, step:, created_at: two_days_ago)
d2 = create(:deployment, step:, created_at: two_days_ago)

travel_to(1.minute.ago) { d2.discard! }
current_release = create(:release, train:, scheduled_at: Time.current)

step.reload
expect(step.active_deployments_for(current_release)).to contain_exactly(step.deployments.first, d1)
end

it "returns deployments that will be available for a new future release" do
two_days_ago = 2.days.ago
two_hours_later = 2.hours.from_now
four_hours_later = 4.hours.from_now
step = travel_to(two_days_ago) { create(:step, :with_deployment, release_platform: release_platform) }
d1 = create(:deployment, step:, created_at: two_days_ago)
d2 = create(:deployment, step:, created_at: two_days_ago)
future_release = create(:release, train:, scheduled_at: two_hours_later, completed_at: four_hours_later)

travel_to(1.minute.ago) do
d1.discard!
d2.discard!
end

d3 = create(:deployment, step:)

expect(step.active_deployments_for(future_release)).to contain_exactly(step.deployments.first, d3)
end
end
end

0 comments on commit 61052b6

Please sign in to comment.