Skip to content

[4.0] Check on the component if the categories support is available #20692

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Jul 13, 2018
Merged

[4.0] Check on the component if the categories support is available #20692

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5ce4b9f
Check on the component if the categories support is available
laoneo Jun 8, 2018
b2f878b
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jun 8, 2018
1b47871
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jun 9, 2018
b80397d
add check
laoneo Jun 9, 2018
a451a2a
Move getAssociations to interface
laoneo Jun 9, 2018
da40e5d
Merge remote-tracking branch 'origin/j4/fix/associations' into j4/fix…
laoneo Jun 9, 2018
7218d80
Revert
laoneo Jun 9, 2018
dbc586a
Change to $cassociations
laoneo Jun 9, 2018
3a3704c
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jun 9, 2018
534e870
CS
laoneo Jun 9, 2018
aee0ad0
Merge remote-tracking branch 'origin/j4/fix/associations' into j4/fix…
laoneo Jun 9, 2018
fb82795
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jun 26, 2018
720f944
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jun 26, 2018
f26ebdd
Merge branch '4.0-dev' into j4/fix/associations
infograf768 Jul 13, 2018
50aea58
Merge branch '4.0-dev' into j4/fix/associations
laoneo Jul 13, 2018
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
18 changes: 13 additions & 5 deletions administrator/components/com_categories/Model/CategoriesModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

defined('_JEXEC') or die;

use Joomla\CMS\Association\AssociationServiceInterface;
use Joomla\CMS\Categories\CategoriesServiceInterface;
use Joomla\CMS\Factory;
use Joomla\CMS\MVC\Factory\MVCFactoryInterface;
Expand Down Expand Up @@ -326,15 +327,22 @@ public function getAssoc()
if (!$assoc || !$component || !$cname)
{
$assoc = false;
return $assoc;
Copy link
Member

@carlitorweb carlitorweb Jun 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

[CS] Add an empty line before the return

}
else
{
$hname = $cname . 'HelperAssociation';
\JLoader::register($hname, JPATH_SITE . '/components/' . $component . '/helpers/association.php');

$assoc = class_exists($hname) && !empty($hname::$category_association);
$component = Factory::getApplication()->bootComponent($component);

if ($component instanceof AssociationServiceInterface)
{
$assoc = $component->hasAssociationsCategorySupport();
return $assoc;
Copy link
Member

@carlitorweb carlitorweb Jun 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

[CS] Add an empty line before the return

}

$hname = $cname . 'HelperAssociation';
\JLoader::register($hname, JPATH_SITE . '/components/' . $component . '/helpers/association.php');

$assoc = class_exists($hname) && !empty($hname::$category_association);

return $assoc;
}

Expand Down
20 changes: 15 additions & 5 deletions administrator/components/com_categories/Model/CategoryModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@

defined('_JEXEC') or die;

use Joomla\CMS\Association\AssociationServiceInterface;
use Joomla\CMS\Categories\CategoriesServiceInterface;
use Joomla\CMS\Component\ComponentHelper;
use Joomla\CMS\Factory;
use Joomla\CMS\Helper\TagsHelper;
use Joomla\CMS\Language\Associations;
use Joomla\CMS\Language\LanguageHelper;
Expand Down Expand Up @@ -1281,15 +1284,22 @@ public function getAssoc()
if (!$assoc || !$component || !$cname)
{
$assoc = false;
return $assoc;
Copy link
Member

@carlitorweb carlitorweb Jun 8, 2018

Choose a reason for hiding this comment

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

[CS] Add an empty line before the return

}
else
{
$hname = $cname . 'HelperAssociation';
\JLoader::register($hname, JPATH_SITE . '/components/' . $component . '/helpers/association.php');

$assoc = class_exists($hname) && !empty($hname::$category_association);
$component = Factory::getApplication()->bootComponent($component);

if ($component instanceof AssociationServiceInterface)
{
$assoc = $component->hasAssociationsCategorySupport();
return $assoc;
Copy link
Member

@carlitorweb carlitorweb Jun 8, 2018

Choose a reason for hiding this comment

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

[CS] Add an empty line before the return

}

$hname = $cname . 'HelperAssociation';
\JLoader::register($hname, JPATH_SITE . '/components/' . $component . '/helpers/association.php');

$assoc = class_exists($hname) && !empty($hname::$category_association);

return $assoc;
}
}
9 changes: 9 additions & 0 deletions libraries/src/Association/AssociationServiceInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,13 @@ interface AssociationServiceInterface
* @since 4.0.0
*/
public function getAssociationsExtension(): AssociationExtensionInterface;

/**
* Are categories associations supported.
*
* @return boolean
*
* @since __DEPLOY_VERSION__
*/
public function hasAssociationsCategorySupport();
}
14 changes: 14 additions & 0 deletions libraries/src/Association/AssociationServiceTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

defined('JPATH_PLATFORM') or die;

use Joomla\CMS\Categories\CategoriesServiceInterface;

/**
* Trait to implement AssociationServiceInterface
*
Expand Down Expand Up @@ -51,4 +53,16 @@ public function setAssociationExtension(AssociationExtensionInterface $associati
{
$this->associationExtension = $associationExtension;
}

/**
* Are categories associations supported.
*
* @return boolean
*
* @since __DEPLOY_VERSION__
*/
public function hasAssociationsCategorySupport()
Copy link
Contributor

@mbabker mbabker Jun 8, 2018

Choose a reason for hiding this comment

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

Do you really need this method in the interface/trait? You can't just have code do the instanceof check without relying on an interface to be able to say you can do this instanceof check?

Copy link
Member Author

@laoneo laoneo Jun 8, 2018

Choose a reason for hiding this comment

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

If every component which has associations and categories integrated, does also have associations for the category automatically, then yes. Wasn't sure about it?

Copy link
Contributor

@mbabker mbabker Jun 8, 2018

Choose a reason for hiding this comment

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

I just don't get what the difference is in using $component instanceof CategoriesServiceInterface versus $component->hasAssociationsCategorySupport() is all. Like why does this has method need to be part of an interface? Unless you've got a plan for being able to dynamically toggle this support it just feels like adding an interface method for the sake of having it.

Copy link
Member Author

@laoneo laoneo Jun 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Actually the component has control to decide if categories associations is supported, what the trait does is just to add default behavior. As I said if we can guarantee that when a component implements the AssociationsServiceInterface and Categories that associations for categories is then also working for the component. Then I'm more that happy to remove it. I just don't know that detail. Perhaps @infograf768 can shed some light here.

{
return $this instanceof CategoriesServiceInterface;
}
}