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

[com_fields] Change extension variable to context for custom field groups #13175

Merged
merged 4 commits into from
Dec 18, 2016

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Dec 13, 2016

Summary of Changes

The new field groups (#13019) are created globally per component. It means when a component has more than one context, it is not possible to create groups per context. This pr solves that and allows to create groups per context of fields. A field belongs to a context and with this pr the group as well.

It fixes also an issue where a second asset per group is created when a permission has changed.

Testing Instructions

After a fresh installation, create groups, assign fields to them. All should work as before.

A fresh installation is needed as the database schema has changed..

@Bakual
Copy link
Contributor

Bakual commented Dec 13, 2016

Some background to explain why I chose "extension" instead of "context" for the groups:

  • com_categories basically does the same thing and uses "&extension" as the URL parameter to determine the category context. So I just kept the "standard".
  • "context" is already used in JModelList as a class proeprty ($this->context). There is some chance for confusion if there is an additional context which doesn't relate to the model context.

So imho we should change all "context" to "extension" but Allon disagrees with me here 😄

@@ -197,7 +204,7 @@ protected function getSortFields()
'a.title' => JText::_('JGLOBAL_TITLE'),
'a.access' => JText::_('JGRID_HEADING_ACCESS'),
'language' => JText::_('JGRID_HEADING_LANGUAGE'),
'a.extension' => JText::_('JGRID_HEADING_EXTENSION'),
'a.context' => JText::_('JGRID_HEADING_context'),
Copy link
Contributor

Choose a reason for hiding this comment

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

JGRID_HEADING_contextShould be all uppercase here.

* @copyright Copyright (C) 2005 - 2016 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
defined('_JEXEC') or die;

$app = JFactory::getApplication();
$component = $app->getUserStateFromRequest('com_fields.groups.extension', 'extension', '', 'CMD');
$context = $app->getUserStateFromRequest('com_fields.groups.context', 'context', '', 'CMD');
$component = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

$component isn't populated here and thus the ACL check will break.
Best is probably to move the explode part out of the if clause to the line just before the ACL check.

@ralain
Copy link
Contributor

ralain commented Dec 14, 2016

I have tested this item ✅ successfully on 8a7f5b0


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13175.

@anibalsanchez
Copy link
Contributor

I have tested this item ✅ successfully on 8a7f5b0

Test OK.

Amazing progress!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13175.

@jeckodevelopment
Copy link
Member

RTC based on 2 successful tests


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13175.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 18, 2016
@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.0 milestone Dec 18, 2016
@laoneo
Copy link
Member Author

laoneo commented Dec 18, 2016

Thanks to the testers!

@rdeutz rdeutz merged commit 5610e4e into joomla:staging Dec 18, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 18, 2016
@laoneo laoneo deleted the group-context branch December 18, 2016 20:20
Bakual pushed a commit that referenced this pull request Jan 7, 2017
* MSSQL - update sql for Custom fields (#11833)

MSSQL - update sql for Custom fields (#11833)

* MSSQL - update sql for Custom fields (#11833)

MSSQL - update sql for Custom fields (#11833)

* MSSQL - install sql for Custom fields (#11833)

MSSQL - install sql for Custom fields (#11833)

* minor cs + defaul values

* move hits field on a new line

move hits field on a new line

* removed version, hits fields

removed (version, hits) fields #12674

* removed (version,hits) fields

removed (version,hits) fields #12674

* mssql com_fields#13091

Fixing sql fields #13091

* mssql com_fields#13091

mssql com_fields#13091

* [com_fields] No need for an alias in fields groups. #13115

[com_fields] No need for an alias in fields groups. #13115

* [com_fields] No need for an alias in fields groups. #13115

[com_fields] No need for an alias in fields groups. #13115

* missed comma

missed comma

* missed comma

missed comma

* update for #13175

from extension to context

* update for #13175

from extension to context

* update for #13246

update for #13246

* updated for #13246

updated for #13246

* added the missed DEFAULT

added the missed DEFAULT

* added space before (

added space before (

* added space before (

added space before (
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants