Skip to content

Commit

Permalink
change add to set
Browse files Browse the repository at this point in the history
  • Loading branch information
Dmytro Sokil committed Mar 24, 2016
1 parent 3308f93 commit 70f9129
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
11 changes: 8 additions & 3 deletions src/Sokil/Vast/Ad/Wrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@ class Wrapper extends \Sokil\Vast\Ad
* @param $uri
* @return $this
*/
public function addVASTAdTagURI($uri)
public function setVASTAdTagURI($uri)
{
// get VASTAdTagURI dom node
$VASTAdTagURIDomElement = $this->domElement->getElementsByTagName('VASTAdTagURI')->item(0);

// create VASTAdTagURI-Node
$VASTAdTagURIDomElement = $this->domElement->ownerDocument->createElement('VASTAdTagURI');
$this->domElement->firstChild->appendChild($VASTAdTagURIDomElement);
if(!$VASTAdTagURIDomElement) {
$VASTAdTagURIDomElement = $this->domElement->ownerDocument->createElement('VASTAdTagURI');
$this->domElement->firstChild->appendChild($VASTAdTagURIDomElement);
}

// create VASTAdTagURI-cdata
$cdata = $this->domElement->ownerDocument->createCDATASection($uri);
Expand Down
2 changes: 1 addition & 1 deletion tests/Sokil/Vast/DocumentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function testCreateWrapperAdSection()
$ad1 = $document->createWrapperAdSection()
->setId('ad1')
->setAdSystem('Ad Server Name')
->addVASTAdTagURI('http://entertainmentserver.com/vart.xml');
->setVASTAdTagURI('http://entertainmentserver.com/vart.xml');

$actualXml = str_replace(array("\r", "\n"), '', $document->toString());

Expand Down

3 comments on commit 70f9129

@Psiiirus
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you gonna use "set" for attributes and "add" for nodes?!

@sokil
Copy link
Owner

@sokil sokil commented on 70f9129 Mar 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add used if allowed few nodes in one node, e.g.

<?php
public function addVideoClicksClickTracking($url) {}

will add number of ClickTracking nodes to Linear creative. But VASTAdTagURI may be only one in Wrapper, so it must be set, not added.

This is just name convenience

@Psiiirus
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see ...so i got it wrong ;)

Please sign in to comment.