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

[FEATURE] Add support for groupHomePath feature #6

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

stigfaerch
Copy link
Contributor

Resolves #5

@stigfaerch
Copy link
Contributor Author

I would prefer this :
$groupHomeStorageUid = (int)explode(':', $GLOBALS['TYPO3_CONF_VARS']['BE']['groupHomePath'], 1);
Because second parameter isn't mandatory, I guess, for example, you can have groupHomePath = "1:" and then groupHomePath will be = "/"

The above code seems to always return 1, I guess because of type casting on the array always returned from the explode function.

In the settings it is described that a combined folder identifier is required and looks like this:
[storageUid]:[folderIdentifier]
So I think we must validate that the colon delimiter is present, and that the first value is an integer

There are multiple ways to do this. Another way could be to use the core code you described:
[$groupHomeStorageUid, $groupHomeFilter] = explode(':',$GLOBALS['TYPO3_CONF_VARS']['BE']['groupHomePath'], 2);
And then check if $groupHomeStorageUid > 0 and if $groupHomeFilter is not null.

@stigfaerch
Copy link
Contributor Author

All the code you've added there shouldn't be there, StateBase is a base class without logic, the logic is implemented in the step itself. This code should be in StateCreateFileMount and better use code like this :

[$groupHomeStorageUid, $groupHomeFilter] = explode(':', $GLOBALS['TYPO3_CONF_VARS']['BE']['groupHomePath'], 2);
$groupHomeStorageUid = (int)$groupHomeStorageUid;
$groupHomeFilter = '/' . ltrim($groupHomeFilter, '/');

As it's the code used in TYPO3 core and takes care of empty home path.

The reason I placed it here, is that StateCreateBeGroup also needs access to the data:
'TSconfig' => 'options.defaultUploadFolder = ' . $this->getSiteFolderCombinedIdentifier($context)
I think a better option might be to place it in a separate utility class then?

@stigfaerch
Copy link
Contributor Author

I prefer something like this :

public function process(SiteGeneratorWizard $context): void
{
    if($context->getSiteData()->getGroupHomePath()) {
        // Get file mount id from global 'groupHomePath'
        $mountId =  $this->getFromHomePath();
    }

Now implemented.

@stigfaerch
Copy link
Contributor Author

Here the code must be simple, because the only thing we need is a different folder depending if we use GroupHomePath or not something like :

$basePath = ($context->getSiteData()->getGroupHomePath() ?  $this->getBasePath() : $siteData->getBaseFolderName());

$this->createFolders($siteData, $context, $basePath);

The reason that I created StateCreateGroupHomeFolder as an empty extension of StateCreateFolder, is that the state prerequisites for creating the folder with the groupHomePath is not the same as when creating just by siteTitle.

When using groupHomePath, then the StateCreateBeGroup has to be already executed, as we need the groupId.
On the other hand, when using siteTitle for the folder, the StateCreateFolder is a prerequisite for StateCreateBeGroup, and must not already have been executed. That's why I inserted:
55 = Oktopuce\SiteGenerator\Wizard\StateCreateGroupHomeFolder
And the get_class comparison ensures that it StateCreateFolder is executed at the correct position depending if groupHomePath of siteTitle is used.

I will try to find a solution to find the basePath however.

@stigfaerch
Copy link
Contributor Author

I prefer something like this :

public function process(SiteGeneratorWizard $context): void
{
    if($context->getSiteData()->getGroupHomePath()) {
        // Get file mount id from global 'groupHomePath'
        $mountId =  $this->getFromHomePath();
    }
    else {
        // Create file mount for site
        $mountId = $this->createFileMount($context->getSiteData());
    }

    $context->getSiteData()->setMountId($mountId);
}

Hmm... I already implemented this. But I don't think it makes sense.
The identifier retrieved from createFileMount is a file mount uid. But there is no such thing when we use the groupHomePath. The id we here retrieve from getFromHomePath is the storage uid, which is something else.
Am I not correct?

@stigfaerch
Copy link
Contributor Author

Here the code must be simple, because the only thing we need is a different folder depending if we use GroupHomePath or not something like :
I will try to find a solution to find the basePath however.

@Oktopuce
Inside createFolders we need 3 things which all can be different depending if we use groupHomePath or not:

  • The storageUid
    • When groupHomePath it comes from the ['groupHomePath'] variable
    • When not, it comes from settings
  • The basepath
    • When groupHomePath it comes from the ['groupHomePath'] variable
    • When not, it comes from $siteData BaseDto
  • The site folder
    • When groupHomePath it comes from uid of the created BE user group
    • When not it comes title of $siteData BaseDto

One option would be to provide a combined folder identifier for createFolders and then explode it inside the method to retrieve the values.
We still need the combined folder identifier for stateCreateBeGroup. Maybe it would be an idea to put a method to generate this into an utility class?

@stigfaerch
Copy link
Contributor Author

One option would be to provide a combined folder identifier for createFolders and then explode it inside the method to retrieve the values.

Another option would be to add all the parameters to the method:
protected function createFolders(BaseDto $siteData, int $storageUid, string $basePath, string $siteFolder): void

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.

Add support for the core groupHomePath feature
2 participants