Split developers and organizations #239

Merged
merged 2 commits into from Sep 13, 2012

Conversation

Projects
None yet
3 participants
Collaborator

cursedcoder commented Sep 11, 2012

No description provided.

src/Knp/Bundle/KnpBundlesBundle/Consumer/UpdateBundleConsumer.php
@@ -3,11 +3,12 @@
namespace Knp\Bundle\KnpBundlesBundle\Consumer;
use Knp\Bundle\KnpBundlesBundle\Github\Repo;
+use Knp\Bundle\KnpBundlesBundle\Entity\OwnerManager;
@cursedcoder

cursedcoder Sep 11, 2012

Collaborator

will remove one

Contributor

stloyd commented Sep 11, 2012

IMO you should move: Developer/show.(js|json).twig to layout/generic_show.(js|json).twig and remove same for Organization.

Contributor

stof commented Sep 12, 2012

@cursedcoder you should rebase your branch as the history seems to be messed

src/Knp/Bundle/KnpBundlesBundle/Controller/OrganizationController.php
+
+ public function showAction(Request $request, $name)
+ {
+ if (!$organization = $this->getOrganizationRepository()->findOneByNameWithRepos($name)) {
@stloyd

stloyd Sep 12, 2012

Contributor

$this->getOrganizationRepository() not anymore exists.

src/Knp/Bundle/KnpBundlesBundle/Controller/OrganizationController.php
+
+ $this->highlightMenu('organizations');
+
+ $query = $this->getOrganizationRepository()->queryAllWithBundlesSortedBy($sortField);
@stloyd

stloyd Sep 12, 2012

Contributor

$this->getOrganizationRepository() not anymore exists.

src/Knp/Bundle/KnpBundlesBundle/Controller/OrganizationController.php
+
+ foreach ($paginator as $organization) {
+ $organizations[] = $organization[0];
+ }
@stloyd

stloyd Sep 12, 2012

Contributor

These seems to be wrong.

@cursedcoder

cursedcoder Sep 12, 2012

Collaborator

Check this post from Antoine http://stackoverflow.com/a/8527531

src/Knp/Bundle/KnpBundlesBundle/Controller/OrganizationController.php
+ return $this->redirect($this->generateUrl('organization_show', array('name' => $name)));
+ }
+
+ if (!$organization = $this->getOrganizationRepository()->findOneByName($name)) {
@stloyd

stloyd Sep 12, 2012

Contributor

$this->getOrganizationRepository() not anymore exists.

src/Knp/Bundle/KnpBundlesBundle/Controller/OrganizationController.php
+
+ public function membersAction(Request $request, $name)
+ {
+ if (!$organization = $this->getOrganizationRepository()->findOneByName($name)) {
@stloyd

stloyd Sep 12, 2012

Contributor

$this->getOrganizationRepository() not anymore exists.

src/Knp/Bundle/KnpBundlesBundle/Github/Developer.php
+
+ if ($response instanceof AdvancedUserResponseInterface) {
+ $user->setEmail($response->getEmail());
+ $user->setGravatarHash($response->getProfilePicture());
@stloyd

stloyd Sep 12, 2012

Contributor

You have renamed this method.

src/Knp/Bundle/KnpBundlesBundle/Github/Organization.php
+use Knp\Bundle\KnpBundlesBundle\Entity\Organization as EntityOrganization,
+ Knp\Bundle\KnpBundlesBundle\Repository\OrganizationRepository;
+
+use Github\HttpClient\Exception as GithubException;
@stloyd

stloyd Sep 12, 2012

Contributor

This is wrong.

src/Knp/Bundle/KnpBundlesBundle/Repository/DeveloperRepository.php
+ ->where('d.name = :name')
+ ->setParameter('name', $name)
+ ->getQuery()
+ ->getSingleResult();
@stloyd

stloyd Sep 12, 2012

Contributor

getOneOrNullResult() would be more useful.

src/Knp/Bundle/KnpBundlesBundle/Repository/OrganizationRepository.php
+ ->where('d.name = :name')
+ ->setParameter('name', $name)
+ ->getQuery()
+ ->getSingleResult();
@stloyd

stloyd Sep 12, 2012

Contributor

getOneOrNullResult() would be more useful.

src/Knp/Bundle/KnpBundlesBundle/Repository/OwnerRepository.php
+ ->where('d.name = :name')
+ ->setParameter('name', $name)
+ ->getQuery()
+ ->getSingleResult();
@stloyd

stloyd Sep 12, 2012

Contributor

getOneOrNullResult() would be more useful.

src/Knp/Bundle/KnpBundlesBundle/Repository/OrganizationRepository.php
+ {
+ $qb = $this->createQueryBuilder('u')
+ ->select('u, SIZE(u.bundles) bundles, SIZE(u.members) members')
+ ->orderBy('bundles', 'ASC')
@stloyd

stloyd Sep 12, 2012

Contributor

This is wrong.

src/Knp/Bundle/KnpBundlesBundle/Repository/OrganizationRepository.php
+ public function queryAllWithBundlesSortedBy($field)
+ {
+ $qb = $this->createQueryBuilder('u')
+ ->select('u, SIZE(u.bundles) bundles, SIZE(u.members) members')
@stloyd

stloyd Sep 12, 2012

Contributor

IMO you don't need to call counts if you will not sort by them.

+
+ if ($format == 'html') {
+ return $this->redirect($this->generateUrl('organization_show', array('name' => $name)));
+ }
@stloyd

stloyd Sep 12, 2012

Contributor

Change this same way as others (first compare, then then db call)

src/Knp/Bundle/KnpBundlesBundle/Entity/OwnerManager.php
+use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
+
+use Github\Client,
+ Github\HttpClient\Exception as GithubException;
@stloyd

stloyd Sep 13, 2012

Contributor

This not exists.

@cursedcoder

cursedcoder Sep 13, 2012

Collaborator

and not used

src/Knp/Bundle/KnpBundlesBundle/Entity/Owner.php
+ public function isEqualTo(Owner $user)
+ {
+ return $user->getName() === $this->getName();
+ }
@stloyd

stloyd Sep 13, 2012

Contributor

IMO this should be implemented on Developer and Organization entities, not here, with checks for equality of class types.

stloyd added a commit that referenced this pull request Sep 13, 2012

@stloyd stloyd merged commit 07d6b22 into KnpLabs:master Sep 13, 2012

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment