diff --git a/docs/migration.txt b/docs/migration.txt index 7aeee4385..3cef42152 100644 --- a/docs/migration.txt +++ b/docs/migration.txt @@ -188,3 +188,9 @@ WebGUI::Shop::Vendor ==================== Object properties are no longer written to the database when an object is created from scratch. The write method needs to be called. + +WebGUI::Shop::AddressBook +========================= +Since create is now really new, there is no way to create an address book for an arbitrary userId. To work around this, +update the address book with the new userId after it has been created. + diff --git a/lib/WebGUI/Shop/AddressBook.pm b/lib/WebGUI/Shop/AddressBook.pm index 65c9d4c71..b2565b454 100644 --- a/lib/WebGUI/Shop/AddressBook.pm +++ b/lib/WebGUI/Shop/AddressBook.pm @@ -97,20 +97,28 @@ around BUILDARGS => sub { if (! (blessed $session && $session->isa('WebGUI::Session')) ) { WebGUI::Error::InvalidObject->throw(expected=>"WebGUI::Session", got=>(ref $session), error=>"Need a session."); } + if ($session->user->isVisitor) { + WebGUI::Error::InvalidParam->throw(error=>"Visitor cannot have an address book."); + } my ($addressBookId) = $class->_init($session); $properties->{addressBookId} = $addressBookId; + $properties->{userId} = $session->user->userId; return $class->$orig($properties); } my $session = shift; if (! (blessed $session && $session->isa('WebGUI::Session'))) { WebGUI::Error::InvalidObject->throw(expected=>"WebGUI::Session", got=>(ref $session), error=>"Need a session."); } + if ($session->user->isVisitor) { + WebGUI::Error::InvalidParam->throw(error=>"Visitor cannot have an address book."); + } my $argument2 = shift; if (!defined $argument2) { my ($addressBookId) = $class->_init($session); my $properties = {}; $properties->{session} = $session; $properties->{addressBookId} = $addressBookId; + $properties->{userId} = $session->user->userId; return $class->$orig($properties); } ##Look up one in the db @@ -135,7 +143,7 @@ sub _init { my $class = shift; my $session = shift; my $addressBookId = $session->id->generate; - $session->db->write('insert into addressBook (addressBookId) values (?)', [$addressBookId]); + $session->db->write('insert into addressBook (addressBookId, userId) values (?,?)', [$addressBookId, $session->user->userId]); return ($addressBookId); } @@ -219,32 +227,19 @@ sub appendAddressFormVars { #------------------------------------------------------------------- -=head2 create ( session, userId ) +=head2 create ( session ) -Constructor. Creates a new address book for this user. +Deprecated, left as a stub for existing code. Use L instead. =head3 session A reference to the current session. -=head3 userId - -The userId for the user. Throws an exception if it is Visitor. Defaults to the session -user if omitted. - =cut sub create { - my ($class, $session, $userId) = @_; - unless (defined $session && $session->isa("WebGUI::Session")) { - WebGUI::Error::InvalidObject->throw(expected=>"WebGUI::Session", got=>(ref $session), error=>"Need a session."); - } - $userId ||= $session->user->userId; - if ($userId eq '1') { - WebGUI::Error::InvalidParam->throw(error=>"Visitor cannot have an address book."); - } - my $id = $session->db->setRow("addressBook", "addressBookId", {addressBookId=>"new", userId=>$userId}); - return $class->new($session, $id); + my ($class, $session) = @_; + return $class->new($session); } #------------------------------------------------------------------- @@ -257,7 +252,6 @@ Deletes this address book and all addresses contained in it. sub delete { my ($self) = @_; - my $myId = id $self; foreach my $address (@{$self->getAddresses}) { $address->delete; } @@ -418,41 +412,6 @@ sub missingFields { #------------------------------------------------------------------- -=head2 new ( session, addressBookId ) - -Constructor. Instanciates an addressBook based upon a addressBookId. - -=head3 session - -A reference to the current session. - -=head3 addressBookId - -The unique id of an address book to instanciate. - -=cut - -sub new { - my ($class, $session, $addressBookId) = @_; - unless (defined $session && $session->isa("WebGUI::Session")) { - WebGUI::Error::InvalidObject->throw(expected=>"WebGUI::Session", got=>(ref $session), error=>"Need a session."); - } - unless (defined $addressBookId) { - WebGUI::Error::InvalidParam->throw(error=>"Need an addressBookId."); - } - my $addressBook = $session->db->quickHashRef('select * from addressBook where addressBookId=?', [$addressBookId]); - if ($addressBook->{addressBookId} eq "") { - WebGUI::Error::ObjectNotFound->throw(error=>"No such address book.", id=>$addressBookId); - } - my $self = register $class; - my $id = id $self; - $session{ $id } = $session; - $properties{ $id } = $addressBook; - return $self; -} - -#------------------------------------------------------------------- - =head2 newByUserId ( session, userId ) Constructor. Creates a new address book for this user if they don't have one. In any case returns a reference to the address book. @@ -499,7 +458,7 @@ sub newByUserId { } else { # nope create one for the user - return $class->create($session); + return $class->new($session); } } diff --git a/t/Shop/AddressBook.t b/t/Shop/AddressBook.t index 3938f747c..891fa522e 100644 --- a/t/Shop/AddressBook.t +++ b/t/Shop/AddressBook.t @@ -32,7 +32,7 @@ my $session = WebGUI::Test->session; #---------------------------------------------------------------------------- # Tests -plan tests => 26; +plan tests => 23; #---------------------------------------------------------------------------- # put your tests here @@ -60,17 +60,7 @@ cmp_deeply( 'new takes exception to not giving it a session object', ); -eval { $book = WebGUI::Shop::AddressBook->new($session); }; -$e = Exception::Class->caught(); -isa_ok($e, 'WebGUI::Error::InvalidParam', 'new takes exception to not giving it a addressBookId'); -cmp_deeply( - $e, - methods( - error => 'Need an addressBookId.', - ), - 'new takes exception to not giving it a addressBook Id', -); - +$session->user({userId => 3}); eval { $book = WebGUI::Shop::AddressBook->new($session, 'neverAGUID'); }; $e = Exception::Class->caught(); isa_ok($e, 'WebGUI::Error::ObjectNotFound', 'new takes exception to not giving it an existing addressBookId'); @@ -82,31 +72,12 @@ cmp_deeply( ), 'new takes exception to not giving it a addressBook Id', ); - - -####################################################################### -# -# create -# -####################################################################### - -eval { $book = WebGUI::Shop::AddressBook->create(); }; -$e = Exception::Class->caught(); -isa_ok($e, 'WebGUI::Error::InvalidParam', 'create takes exception to not giving it a session object'); -cmp_deeply( - $e, - methods( - error => 'Need a session.', - expected => 'WebGUI::Session', - got => '', - ), - 'create takes exception to not giving it a session object', -); - $session->user({userId => 1}); -eval { $book = WebGUI::Shop::AddressBook->create($session); }; + + +eval { $book = WebGUI::Shop::AddressBook->new($session); }; $e = Exception::Class->caught(); -isa_ok($e, 'WebGUI::Error::InvalidParam', 'create takes exception to making an address book for Visitor'); +isa_ok($e, 'WebGUI::Error::InvalidParam', 'new takes exception to making an address book for Visitor'); cmp_deeply( $e, methods( @@ -116,22 +87,23 @@ cmp_deeply( ); $session->user({userId => 3}); -$book = WebGUI::Shop::AddressBook->create($session); -isa_ok($book, 'WebGUI::Shop::AddressBook', 'create returns the right kind of object'); +$book = WebGUI::Shop::AddressBook->new($session); +isa_ok($book, 'WebGUI::Shop::AddressBook', 'new returns the right kind of object'); isa_ok($book->session, 'WebGUI::Session', 'session method returns a session object'); is($session->getId, $book->session->getId, 'session method returns OUR session object'); -ok($session->id->valid($book->getId), 'create makes a valid GUID style addressBookId'); +ok($session->id->valid($book->getId), 'new makes a valid GUID style addressBookId'); -is($book->get('userId'), 3, 'create uses $session->user to get the userid for this book'); +is($book->get('userId'), 3, 'new uses $session->user to get the userid for this book'); +is($book->userId, 3, '... testing direct accessor'); my $bookCount = $session->db->quickScalar('select count(*) from addressBook'); is($bookCount, 1, 'only 1 address book was created'); -my $alreadyHaveBook = WebGUI::Shop::AddressBook->create($session); -isnt($book->getId, $alreadyHaveBook->getId, 'creating an addressbook as visitor, even when you already have one, always returns a new one'); +my $alreadyHaveBook = WebGUI::Shop::AddressBook->new($session); +isnt($book->getId, $alreadyHaveBook->getId, 'creating an addressbook, even when you already have one, always returns a new one'); ####################################################################### # @@ -177,15 +149,16 @@ $book->update({ lastShipId => $address1->getId, lastPayId => $address2->getId}); cmp_deeply( $book->get(), { - userId => ignore(), - addressBookId => ignore(), + userId => ignore(), + addressBookId => ignore(), defaultAddressId => ignore(), }, - 'update updates the object properties cache' + 'update does not add new properties to the object' ); my $bookClone = WebGUI::Shop::AddressBook->new($session, $book->getId); +delete $book->{_addressCache}; cmp_deeply( $bookClone, $book, @@ -223,13 +196,14 @@ my $otherSession = WebGUI::Test->newSession; my $mergeUser = WebGUI::User->create($otherSession); WebGUI::Test->addToCleanup($mergeUser); $otherSession->user({user => $mergeUser}); -my $adminBook = WebGUI::Shop::AddressBook->create($otherSession); +my $adminBook = WebGUI::Shop::AddressBook->new($otherSession); WebGUI::Test->addToCleanup($adminBook); my $goodAddress = $adminBook->addAddress({label => 'first'}); my $session2 = WebGUI::Test->newSession; $session2->user({user => $mergeUser}); my $bookAdmin = WebGUI::Shop::AddressBook->newByUserId($session2); +WebGUI::Test->addToCleanup($bookAdmin); cmp_bag( [ map { $_->getId } @{ $bookAdmin->getAddresses } ],