Hello,
I apologize for the length of the topic but I didn’t want to open others with the same argument (and sorry for my english )
In serveral years i made some change and customization for Collection and “fixed” some issue.
I share with you what I have done, for opinion and to find out, especially for changes and customizations, if they create conflicts or if there are better solutions
I am on Collection 4.1.1 and Modx 3.1.0
ISSUE
Issue with lexicon topic - ContentTypes
In collection view setting, for content Types, the value of entry of lexicon topic dont’ appear, only see the index. This beacause don’t load the lexicon topic
Solution
On file core\components\collections\src\Processors\Extra\ GetContentTypes.php at line 10. add collections:default
public $languageTopics = ['content_type','collections:default'];
Issue with icon e class_key
In the prepareIcons function, the old nomenclature for class_key (e.g. modResource) is considered and not the new one (e.g. modx\revolution\modResource)
I didn’t change everything but only the part about particular types of resources
Solution
On file core\components\collections\src\Processors\Resource\ GetList.php for function prepareIcons (about line 624)
i change switch
`$classKeyNew = strtolower($resourceArray['class_key']);
switch ($classKeyNew) {
case 'modx\revolution\modweblink':
$iconCls[] = $this->iconMap['weblink'];
break;
case 'modx\revolution\modsymlink':
$iconCls[] = $this->iconMap['symlink'];
break;
case 'modx\revolution\modstaticresource':
$iconCls[] = $this->iconMap['staticresource'];
break;
}`
Issue for localizzation (Lexicon Topic)
In Collection view the label for “Default for Template” is a text and not a entry of lexicon topic
Solution
Add entry in lexicon topic default
$_lang['collections.template.defaultfortemplates'] = 'Default for Templates';
On file assets\components\collections\js\mgr\widgets\template\ template.grid.js on line 53 change text with lexicon
header: _('collections.template.defaultfortemplates')
Issue with preview of resource inside a selection
The button preview for a resource inside a selection don’t work, the function handleButtons is wrong
Solution
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.selection.js
simply i copy the same function from collections.grid.resources.js
Issue with multiple nested selection and resources
If link resource in nested selection and open button the resource don’t linked
Solution
i don’t know if this is the generale soluzion, but for my logic explained below, it works
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.resource.js
I remove the folderGet form getPageUrl
return collections.getPageUrl(MODx.request.a, 'id=' + data.id + selection + collectionGet );
Issue (Change) for RefreshTree, selection
When i change status of a resource linked in selection, the resource’s tree don’t refresh and the status don’t syncronize
Solution
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.selection.js
add Ext.getCmp('modx-layout').refreshTrees(); for listeners success function in function deleteChild, removeChild, deleteSelected
CHANGE
In my logic inside a collection i want to see nested collection and only the resource inside must have the open button.
Change: set icon for resource linked in selection
Now no icon appear, i change this with icon-chain for alla resource linked
For make this
On file core\components\collections\src\Processors\Selection\ GetList.php for function prepareRow (586)
i add $resourceArray['icons'] = 'icon icon-chain'; in the end
Change: set icon for collection and selection
For make this
On file core\components\collections\src\Processors\Resource\ GetList.php in function prepareIcons (about line 624)
i add just before $resourceArray['icons'] = implode(' ', $iconCls); , the following code
if ($resourceArray['class_key'] == 'Collections\Model\SelectionContainer' ) {
$iconCls[] = 'selectioncontainer';
}
if ($resourceArray['class_key'] == 'Collections\Model\CollectionContainer' ) {
$iconCls[] = 'collectioncontainer';
}
Change: open button only for resource
For my logic only resource must have open button
For make this
On file core\components\collections\src\Processors\Resource\ GetList.php in function prepareActions (about line 508)
I change case for switch “open”
case 'open':
//MABOL inserito if che mostra tasto open solo se NON è una collezione/Selezione
if ($resourceArray['class_key'] != 'Collections\Model\CollectionContainer' && $resourceArray['class_key'] != 'Collections\Model\SelectionContainer') {
$resourceArray['actions'][] = $this->actions['open'];
}
break;
Change: SUB COLLECTION
I make possible to see the nested sub collections and other
Change: see sub collection inside collection
I create a new sistem setting mostra_sub_collections to decide whether to see them or not inside other collection
On file core\components\collections\src\Processors\Resource\ GetList.php
in function prepareQueryBeforeCount (about line 293)
I change last “where” (with sistem setting)
if (!$this->modx->getOption('mostra_sub_collections', null, false))
{
$c->where([
'class_key:!=' => CollectionContainer::class,
// "NOT EXISTS (SELECT 1 FROM {$this->modx->getTableName('modResource')} r WHERE r.parent = modResource.id)"
]);
}
Change: see sub collection in resourc’s tree
I create a new sistem setting mostra_sub_collections_tree to decide whether to see them or not on resource’s (only for nested collection)
For make this
On file core\components\collections\src\Processors\Events\ OnBeforeDocFormSave.php
For function run i change if (line 24) in this (
if ($resource->class_key == CollectionContainer::class) {
//MABOL, aggiunta possibilità di nascondere le collectioni nel Tree (else valore originario)
if ($this->modx->getOption('mostra_sub_collections_tree', null, true)==false && $parent && ($parent->class_key == CollectionContainer::class))
{
$resource->set('show_in_tree', 0);
}else{
$resource->set('show_in_tree', 1);
}
}
For function switchToCollections i change if (line 141) in this (
if ($child->class_key == CollectionContainer::class) {
//MABOL, aggiunta possibilità di nascondere le collectioni nel Tree (else valore originario)
if ($this->modx->getOption('mostra_sub_collections_tree', null, true)==false)
{
$resource->set('show_in_tree', 0);
}else{
$resource->set('show_in_tree', 1);
}
}
On file core\components\collections\src\Processors\Events\ OnResourceBeforeSort.php
For function run i change if (line 38) in this (
if ($resource->class_key == CollectionContainer::class) {
//MABOL, aggiunta possibilità di nascondere le collectioni nel Tree (else valore originario)
if ($this->modx->getOption('mostra_sub_collections_tree', null, true)==false && $parent && ($parent->class_key == CollectionContainer::class))
{
$resource->set('show_in_tree', 0);
}else{
$resource->set('show_in_tree', 1);
}
}
Issue (Change) for RefreshTree, selection
In collection in resource tre when i change status of these, the resource’s tree don’t refresh and the status don’t syncronize
This, obviously, only if mostra_sub_collections_tree is set to true
For make this
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.resources.js add
if(MODx.config.mostra_sub_collections_tree == true){
Ext.getCmp('modx-layout').refreshTrees();
}
for listeners success function in
deleteChild, remoChild, deleteSelected, undeleteSelected, publishSelected, unpublishSelected, publishChild, unpublishChild
Customization: COLLECTION’s “HEADER”
In some of my projects user groups can only see a collection and nothing else.
To make it more understandable I have introduced a header for each collection or selection with a small breadcrumb
i’m not an expert of extjs but this is my implementation
I create a new sistem setting mostra_cs_testata to decide whether to see them or not the collection’header
and then
Create a new processor file testata.php inside Extra
core\components\collections\src\Processors\Extra\ Testata.php
namespace Collections\Processors\Extra;
use MODX\Revolution\modResource;
use MODX\Revolution\Processors\Processor;
class Testata extends Processor
{
public function process()
{
$collection = (int)$this->getProperty('collection', 0);
$selection = (int)$this->getProperty('selection', 0);
if ($collection <= 0) {
return $this->failure();
}
$resource = $this->modx->getObject(modResource::class, $collection);
if (!$resource) {
return $this->failure();
}
//Discrimino a seconda che sia selezione oppure no per icona
$resource->published?$classPub ='pubblicata':$classPub ='ritirata';
if($selection)
{
$icona ='<i class="icon selectioncontainer"></i>';
}else{
$icona ='<i class="icon collectioncontainer"></i>';
}
//Contesto
$contextKey = $resource->context_key;
$context = $this->modx->getContext( $contextKey );
$contextName = $context->name;
//Prima creo menu
//Prima parte del menu il contesto con la sua icona
$menu = '<span class="eleMenu contesto"><i class="icon tree-context"></i>'.$contextName.'</span>';
$pids = $this->modx->getParentIds($collection, 10, array('context' => $contextKey));
$gids = array_reverse($pids);
foreach($gids as $gid)
{
if($gid == 0){continue;}
$genit = $this->modx->getObject(modResource::class, $gid);
if (!$genit){continue;}
$genit->published?$classPubGen ='pubblicata':$classPubGen ='ritirata';
$genit->hidemenu?$classMenuGen ='nomenu':$classMenuGen ='simenu';
$menu .= '<span class="eleMenu '.$classPubGen.' '.$classMenuGen.'">'.$genit->pagetitle.'</span>';
}
//Ultimo elemento del menu la sua icona
$menu .= '<span class="ultimo '.$classPub.'">'.$icona.'</span>';
$RisAjax [] = $menu;
//Adesso nome a cui aggiungo icona
$nome = $icona;
$nome .= $resource->pagetitle;
$RisAjax [] = $nome;
return $this->outputArray($RisAjax);
}
}
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.resources.js
i add, after window.history.replaceState({}, '', window.location.href); (line 52) , this
if(MODx.config.mostra_cs_testata == true)
{
this.getTestataCollection(config);
}
Then i introduce 2 new function (getTestataCollection printTestataCollection) near the end
,getTestataCollection: function(config) {
var collection = parseInt(MODx.request.id);
MODx.Ajax.request({
url: MODx.config.connector_url
,params: {
action: 'Collections\\Processors\\Extra\\Testata'
,collection: collection
,selection: 0
}
,listeners: {
success: {
fn: function(r) {
this.printTestataCollection(r);
},scope: this
},
failure: {
fn: function(){
this.store.removeAll();
this.currentFolder = collections.template.parent;
this.getStore().baseParams.parent = collections.template.parent;
this.getBottomToolbar().changePage(1);
},
scope: this
}
}
});
}
,printTestataCollection: function(r) {
this.testata = ({
items : [{
html: '<div class="titoloTestataCS">'+r.results[1]+'</div>'+
'<div class="menuTestataCS">'+r.results[0]+'</div>'
,bodyCssClass: 'contentTestataCS'
,anchor: '100%'
}]
});
this.getTopToolbar().insert(1,this.testata);
}
On file assets\components\collections\js\mgr\widgets\category\ collections.grid.selection.js
add only function getTestataCollection with parameter selection to 1
,getTestataCollection: function(config) {
var collection = parseInt(MODx.request.id);
MODx.Ajax.request({
url: MODx.config.connector_url
,params: {
action: 'Collections\\Processors\\Extra\\Testata'
,collection: collection
,selection: 1
}
,listeners: {
success: {
fn: function(r) {
this.printTestataCollection(r);
},scope: this
},
failure: {
fn: function(){
this.store.removeAll();
this.currentFolder = collections.template.parent;
this.getStore().baseParams.parent = collections.template.parent;
this.getBottomToolbar().changePage(1);
},
scope: this
}
}
});
}
That’s All
While waiting for your opinions, improvements and error report, I leave you with some images that make everything clearer
Sorry for bump, but in a single post it wasn’t possible, I got an error 403 forbidden
If you find a bug in a MODX extra and you know how to fix it, then it’s best to create a pull request on Github with the fixed code.
I see that you created a issue on Github, but with a pull request it’s much easier to see what files you changed and the exact code changes you made.
Also, if there are different bugs, create a new pull request for each bug.
It’s better to avoid such long posts (or Github issues) that contain multiple bug fixes and even new features. It gets quite overwhelming for someone trying to understand what exactly you did, and the (most likely) result is, that it will just be ignored.
If you feel that these changes are something that could be generally useful for other users of the extra as well, then also create a pull request for them.
Otherwise maybe create your own fork of the Github repository where you combine all the code changes you made in one place. This would be much more readable (and uselful), than a write-up of your changes in a forum post like this.
The purpose of separate PRs for each fix is that it simplifies testing and reduces the risk of introducing new bugs. It’s far better to have 5 or 10 individual fixes for separate bugs (even if they are in the same topic or around the same component). If there is a bug that is found that was missed in testing, you can then revert that single PR and the rest can live on; otherwise, a big pile of fixes in a single commit means you have to revert all the fixes and then pull out the one you think was the issue and then recommit the rest.
For any changes or improvements, the separation helps in explaining the reasoning why the changes are important and add value to the Extra and allows the author to ask any pertinent questions about what you might not have thought of.
@halftrainedharry & @smashingred … all of this is very true - but remember that not everyone is proficient with Github’s murky workings.
I use Github to manage the codebase for many of my projects - but I’ve never understood the pull request process and I’ve made a complete arse of myself trying to on several occasions!
Raising issues is about as far as I go and I think it might be a little unfair to assume anyone who can articulate a problem will always be comfortable with Github and its idiosyncrasies.
For all I know @SilverMabol may be a master of PRs - but you get the idea 
hope I’m not speaking out of turn.
Also - @SilverMabol - thanks for taking the time to share.
You nailed the problem. 
I’m not a master, i’m a super … dummie, i hate PRs
Sorry.
I will try, when I have time, to also study the PRs if it is the only way to get fixes or new changes approved
It’s not “the only way” but it would simplify the process very much.
As you already made the changes to the code files, if would be much simpler to commit this changes to a PR, than typing them out like this
and then the author of the extra has to navigate to each file and make the changes again (based on the provided text) to test it.
I’m aware of that, and it’s ok to just create an issue if you find a bug.
But when you already figured out what files and lines of code need to be changed to fix a bug, it’s much preferable to have a PR.
To create a PR, it can help to have a code editor that has buttons and menu items for Git source control (so you don’t have to write the Git commands by hand in the console).
(Repeat step 3-6 for additional bugs)
This is fair. We certainly don’t expect everyone who can find the issue to be able to submit PRs. It’s not second nature to me either. I was conveying the important message that if one were going to submit PRs, they be broken up for the reasons outlined. But if you’re not able, that’s not a problem at all. Do your best to help make the software better in any way you’re able.
As an aside, if the code you see is in the unbuilt source code in the repo, it may not even be necessary to run Revo locally to create a branch. It may be possible to do it directly here in GitHub after you clone a repo to your account.
Otherwise, including the notes and code changes should be sufficient.
Thanks so much for working to help make Revo better, @SilverMabol and @dejaya!
I promise I will study PRs as soon as I have time
Now, i dont’ clone the fork in local machine, simple change file manually in branch created.
I’m ready to error
I created PRs for each issue/change/customization.
Tell me bad format, bad netiquette, error but not bad English
