Skip to content

Commit

Permalink
Assign suffix to changes fetched from the database.
Browse files Browse the repository at this point in the history
This is so that `revert` will find the correct script for reworked changes.
There might be an argument to be made that this should also come from the
database, but since the suffix comes from a tag, and the tag may have been
changed, this seems safer. Besides, if the change is not in the plan, is the
revert script going to be present, either?

OTOH, maybe it would be better, should it come up in the future, to store the
ID of the previous version of a change, and when we see that, find the
previous version of the change, then the first tag after that change, and
assign the suffix. This would allow it to use the same suffix as was present
when the reworked change was deployed.

Either way, the tag could have been changed, which will cause no end to
confusion. This was the simpler fix for now. Will revisit in the future if it
becomes a problem.

Closes #67.
  • Loading branch information
theory committed Jan 4, 2013
1 parent 7f9ef34 commit 45996d2
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
8 changes: 8 additions & 0 deletions lib/App/Sqitch/Engine.pm
Expand Up @@ -608,6 +608,14 @@ sub _load_changes {
map {
my $tags = $_->{tags} || [];
my $c = App::Sqitch::Plan::Change->new(%{ $_ }, plan => $plan );

# Assign a suffix from the planned change.
if (my $pc = $plan->get( $_->{id} )) {
my $suffix = $pc->suffix;
$c->suffix( $suffix ) if length $suffix;
}

# Add tags.
$c->add_tag(
App::Sqitch::Plan::Tag->new(name => $_, plan => $plan, change => $c )
) for map { s/^@//; $_ } @{ $tags };
Expand Down
19 changes: 16 additions & 3 deletions t/engine.t
Expand Up @@ -4,7 +4,7 @@ use strict;
use warnings;
use 5.010;
use utf8;
use Test::More tests => 541;
use Test::More tests => 543;
#use Test::More 'no_plan';
use App::Sqitch;
use App::Sqitch::Plan;
Expand Down Expand Up @@ -310,15 +310,28 @@ for my $spec (
} @{ $args }], "Should load changes with $desc";
}


# Make sure it picks up the suffix from the plan.
my $change = $plan->change_at(1);
$change->suffix('foo');
ok my ($loaded) = $engine->_load_changes({
id => $change->id,
name => $change->name,
project => 'something',
note => 'Whatever',
planner_name => 'Barack Obama',
planner_email => 'bo@whitehouse.gov',
timestamp => $now,
}), 'Load a change corresponding to a planned change with suffix';
is $loaded->suffix, 'foo', 'It should have picked up the suffix from the plan';
$change->suffix('');

##############################################################################
# Test deploy_change and revert_change.
ok $engine = App::Sqitch::Engine::whu->new( sqitch => $sqitch ),
'Create a subclass name object again';
can_ok $engine, 'deploy_change', 'revert_change';

my $change = App::Sqitch::Plan::Change->new( name => 'users', plan => $sqitch->plan );
$change = App::Sqitch::Plan::Change->new( name => 'users', plan => $sqitch->plan );

ok $engine->deploy_change($change), 'Deploy a change';
is_deeply $engine->seen, [
Expand Down

0 comments on commit 45996d2

Please sign in to comment.