Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ClonePagesCommand #514

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 164 additions & 0 deletions Command/ClonePagesCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
<?php

/*
* This file is part of the Sonata package.
*
* (c) Thomas Rabaix <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Application\Sonata\PageBundle\Command;

use Sonata\PageBundle\Command\BaseCommand as BaseCommand;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

/**
* Clone Pages.
*
* @author Armin Weihbold <[email protected]>
*/
class ClonePagesCommand extends BaseCommand
{
/**
Copy link
Member

Choose a reason for hiding this comment

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

Bad indent.

* {@inheritdoc}
*/
public function configure()
Copy link
Member

Choose a reason for hiding this comment

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

Still 2 spaces on function declaration and brackets. Need 4.

{
$this->setName('sonata:page:clone');
$this->setDescription('Clone pages');
$this->addOption('sourcesite', null, InputOption::VALUE_REQUIRED, 'Source Site id', null);
Copy link
Member

Choose a reason for hiding this comment

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

IMO source-site would be better

Copy link
Member

Choose a reason for hiding this comment

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

same for destsite -> dest-site

$this->addOption('destsite', null, InputOption::VALUE_REQUIRED, 'Dest Site id', null);
Copy link
Member

Choose a reason for hiding this comment

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

IMO dest-site would be better

$this->addOption('prefix', null, InputOption::VALUE_REQUIRED, 'title prefix', null);
Copy link
Member

Choose a reason for hiding this comment

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

description should either be lowercase or uppercase, not mixed

$this->addOption('only-hybrid', null, InputOption::VALUE_OPTIONAL, 'only clone hybrid pages', null);
$this->addOption('additional', null, InputOption::VALUE_OPTIONAL, 'clone additional pages (csv ids)', null);
$this->addOption('base-console', null, InputOption::VALUE_OPTIONAL, 'Base symfony console command', 'php app/console');
}

/**
* {@inheritdoc}
*/
public function execute(InputInterface $input, OutputInterface $output)
{
if (!$input->getOption('sourcesite')) {
$output->writeln('Please provide an <info>--sourcesite=SITE_ID</info> option\n');

$output->writeln(sprintf(' % 5s - % -30s - %s', 'ID', 'Name', 'Url'));

foreach ($this->getSiteManager()->findBy(array()) as $site) {
Copy link
Member

Choose a reason for hiding this comment

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

Use findAll() instead.

$output->writeln(sprintf(' % 5s - % -30s - %s', $site->getId(), $site->getName(), $site->getUrl()));
}

return;
}

if (!$input->getOption('destsite')) {
$output->writeln('Please provide an <info>--destsitesite=SITE_ID</info> option\n');
Copy link
Member

Choose a reason for hiding this comment

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

\n Will not work here. Use double quotes.

Copy link
Member

Choose a reason for hiding this comment

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

destsitesite ... duplicate site

Copy link
Member

Choose a reason for hiding this comment

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

BTW, use PHP_EOL instead of \n.

Copy link
Member

Choose a reason for hiding this comment

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

After reflexion, why adding a new line?

writeln is already doing this.


$output->writeln(sprintf(' % 5s - % -30s - %s', 'ID', 'Name', 'Url'));

foreach ($this->getSiteManager()->findBy(array()) as $site) {
Copy link
Member

Choose a reason for hiding this comment

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

Use findAll() instead.

$output->writeln(sprintf(' % 5s - % -30s - %s', $site->getId(), $site->getName(), $site->getUrl()));
}

return;
}

if (!$input->getOption('prefix')) {
$output->writeln('Please provide a <info>--prefix=PREFIX</info> option\n');

return;
}

if ($input->getOption('only-hybrid')) {
$output->writeln('Only cloning hybrid pages.\n');
Copy link
Member

Choose a reason for hiding this comment

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

\n Will not work here. Use double quotes.

$hybridOnly = true;
} else {
$hybridOnly = false;
}

if ($input->getOption('additional')) {
// TODO: sanity check input
Copy link
Member

Choose a reason for hiding this comment

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

wrong indention of the comment

$output->writeln('Cloning additional pages');
$additional_page_ids = preg_split('/,/', $input->getOption('additional'));
for ($i = 0; $i < count($additional_page_ids); ++$i) {
$additional_page_ids[ $i ] = intval($additional_page_ids[ $i ]);
}
} else {
$additional_page_ids = array();
}

$source_site = $this->getSiteManager()->findBy(array('id' => $input->getOption('sourcesite')));
$dest_site = $this->getSiteManager()->findBy(array('id' => $input->getOption('destsite')));
Copy link
Member

Choose a reason for hiding this comment

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

please use studyCase-variable names instead of snake_case everywhere


$pages = $this->getPageManager()->findBy(array('site' => $source_site[0]));
Copy link
Member

Choose a reason for hiding this comment

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

would use findOneBy() as we can use $source_site variable directly instead of $source_site[0]


$pageClones = array();

$blockClones = array();

// TODO: check if we are missing parents
$output->writeln('Cloning pages\n');
Copy link
Member

Choose a reason for hiding this comment

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

\n Will not work here. Use double quotes.

Bad indent.

foreach ($pages as $page) {
if ($hybridOnly && !$page->isHybrid() && !in_array($page->getId(), $additional_page_ids)) {
continue;
}
$output->writeln(sprintf(' % 4s - % -70s - % 4s', $page->getId(), $page->getTitle(), $page->getParent() ? $page->getParent()->getId() : ''));
$newPage = clone $page;

if ($newPage->getTitle() != '') {
Copy link
Member

Choose a reason for hiding this comment

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

Use strict comparison if possible.

$newPage->setTitle($input->getOption('prefix').$newPage->getTitle());
}

$pageClones[ $page->getId() ] = $newPage;
Copy link
Member

Choose a reason for hiding this comment

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

No spaces.

$newPage->setSite($dest_site[0]);
$this->getPageManager()->save($newPage);

// clone page blocks
$blocks = $this->getBlockManager()->findBy(array('page' => $page));
Copy link
Member

Choose a reason for hiding this comment

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

Bad indent.

foreach ($blocks as $block) {
if ($block->getType() == 'sonata.media.block.gallery') {
Copy link
Member

Choose a reason for hiding this comment

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

Use strict comparison if possible.

continue;
}
$output->writeln(sprintf(' cloning block % 4s ', $block->getId()));
Copy link
Member

Choose a reason for hiding this comment

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

Why spacing before and after? Available for other lines.

Copy link
Member

Choose a reason for hiding this comment

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

i think before could be used for some indention here, but after... don't know

$newBlock = clone $block;
$newBlock->setPage($newPage);
$blockClones[ $block->getId() ] = $newBlock;
Copy link
Member

Choose a reason for hiding this comment

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

No spaces.

$this->getBlockManager()->save($newBlock);
}
}
$output->writeln('Fixing parents and targets\n');
foreach ($pageClones as $page) {
if ($page->getParent()) {
$output->writeln(sprintf('new parent: % 4s - % -70s - % 4s -> % 4s', $page->getId(), $page->getTitle(), $page->getParent() ? $page->getParent()->getId() : '', $pageClones[ $page->getParent()->getId() ]->getId()));
$page->setParent($pageClones[ $page->getParent()->getId() ]);
Copy link
Member

Choose a reason for hiding this comment

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

No spaces.

}

if ($page->getTarget()) {
$output->writeln(sprintf('new target: % 4s - % -70s - % 4s', $page->getId(), $page->getTitle(), $page->getParent() ? $page->getParent()->getId() : ''));
$page->setTarget($pageClones[ $page->getTarget()->getId() ]);
Copy link
Member

Choose a reason for hiding this comment

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

No spaces.

}
$this->getPageManager()->save($newPage, true);
}

$output->writeln('Fixing block parents\n');
foreach ($pageClones as $page) {
$blocks = $this->getBlockManager()->findBy(array('page' => $page));
foreach ($blocks as $block) {
if ($block->getType() == 'sonata.media.block.gallery') {
continue;
}
if ($block->getParent()) {
Copy link
Member

Choose a reason for hiding this comment

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

would add blank line before if-statement

$output->writeln(sprintf('new block parent: % 4s - % 4s', $block->getId(), $blockClones[ $block->getParent()->getId() ]->getId()));
Copy link
Member

Choose a reason for hiding this comment

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

No spaces on array access.

Copy link
Member

Choose a reason for hiding this comment

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

and i would add some line breaks for better readability

$block->setParent($blockClones[ $block->getParent()->getId() ]);
Copy link
Member

Choose a reason for hiding this comment

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

No spaces.

}
$this->getBlockManager()->save($newBlock, true);
Copy link
Member

Choose a reason for hiding this comment

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

would add blank line before $this->getBlockManager()->save($newBlock, true);

}
}

$output->writeln('<info>done!</info>');
}
}