Fix: contentcontainer_id null

[ArchBlood]

Fixes an issue where sometimes a contentcontainer_id is returned as null

humhub/legal#68 (comment)

[luke-]

A content without a ContentContainer should also be able to have a URL.

If there is a problem here, we should rather fix it in the getUrl() method.

[ArchBlood]

A content without a ContentContainer should also be able to have a URL.

If there is a problem here, we should rather fix it in the getUrl() method.

Maybe something like this would be a better solution?

protected static function getContentMetadata($content)
{
    if ($content === null) {
        return null;
    }

    $metadata = [
        'id' => $content->id,
        'guid' => $content->guid,
        'object_model' => $content->object_model,
        'object_id' => $content->object_id,
        'visibility' => (int) $content->visibility,
        'state' => (int) $content->state,
        'archived' => (bool) $content->archived,
        'hidden' => (bool) $content->hidden,
        'pinned' => (bool) $content->pinned,
        'locked_comments' => (bool) $content->locked_comments,
        'created_by' => $content->createdBy ? UserDefinitions::getUserShort($content->createdBy) : null,
        'created_at' => $content->created_at,
        'updated_by' => $content->updatedBy ? UserDefinitions::getUserShort($content->updatedBy) : null,
        'updated_at' => $content->updated_at,
        'scheduled_at' => $content->scheduled_at,
        'contentcontainer_id' => $content->contentcontainer_id,
        'stream_channel' => $content->stream_channel
    ];

    // Get the URL if available
    if ($content->contentcontainer_id !== null) {
        $metadata['url'] = $content->getUrl(true);
    } else {
        $metadata['url'] = null;
    }

    return $metadata;
}

Another approach would be updating the core Content model class getUrl() to something like this;

/**
 * Returns the URL of this content.
 *
 * By default, it returns the URL of the wall entry.
 *
 * Optionally, it's possible to create a custom `getUrl` method in the underlying
 * content model (e.g., Post) to override this behavior.
 * e.g., in case there is no wall entry available for this content.
 *
 * @param bool $scheme Whether to include the scheme (http/https) in the URL. Default is false.
 * @return string The URL of the content.
 * @throws \yii\db\Exception
 * @since 1.15
 */
public function getUrl($scheme = false)
{
    try {
        $polymorphicRelation = $this->getPolymorphicRelation();
        if ($polymorphicRelation && method_exists($polymorphicRelation, 'getUrl')) {
            return $polymorphicRelation->getUrl($scheme);
        }
    } catch (\Throwable $e) {
        Yii::error($e->getMessage(), 'content');
    }

    // Provide a default URL in case no specific URL is available
    if ($this->contentcontainer_id !== null) {
        // If content is associated with a ContentContainer, use PermaLink widget
        return \humhub\modules\content\widgets\PermaLink::widget(['content' => $this]);
    } else {
        // If content is not associated with a ContentContainer, generate a generic URL
        return \yii\helpers\Url::toRoute(['/content/perma', 'id' => $this->id], $scheme);
    }
}

[luke-]

@ArchBlood What is wrong, to handle content without content container via /content/perma? There should be a conditions for such cases: https://github.com/humhub/humhub/blob/master/protected/humhub/modules/content/controllers/PermaController.php#L53

[ArchBlood]

@ArchBlood What is wrong, to handle content without content container via /content/perma? There should be a conditions for such cases: https://github.com/humhub/humhub/blob/master/protected/humhub/modules/content/controllers/PermaController.php#L53

From looking at this, $content->container is used for an object that must not be null then later uses createUrl() to generate the URLs with $urlParams which equals a null array (i.e. ['contentId' => $id]) when dealing with the Custom Pages module;

In the Custom Pages module the following is done where getTargetModel() is returning a null value therefore not allowing for getContentUrl() and throwing the error.

Within the PermaLink widget this isn't a possibility that is why it was used within getUrl() for the Content model class then later in the else we return the preferred method Url::toRoute(['/content/perma', 'id' => $this->id], $scheme) which covers everything else;

So in short getTargetModel() is returned as a nulled object which can't be done as getContentUrl() will throw the error when used in the ContentDefinitions class (i.e. $content->getUrl())

    /**
     * Returns the view url of this page.
     */
    public function getUrl()
    { 
        return $this->getTargetModel()->getContentUrl($this);
    }

[luke-]

@ArchBlood Sorry, I don't understand the problem.

In principle, global content (without ContentContainer) can also have a perma URL via Content::getUrl().

Content::getUrl() must not throw an error here,

If an error occurs here, we should fix the method and not skip the url in the REST module.

[ArchBlood]

@ArchBlood Sorry, I don't understand the problem.

In principle, global content (without ContentContainer) can also have a perma URL via Content::getUrl().

Content::getUrl() must not throw an error here,

If an error occurs here, we should fix the method and not skip the url in the REST module.

That would be rather hard to track down, currently I can only confirm that the issue occurs from the Custom Pages module which would be the following snippet;

Custom Pages

I've only gotten as far as here when it comes to why a null is returned, what I find weird is that I don't have this error if I directly use Content but does happen if I use ContentDefinitions::getContent() which uses getContentMetadata() so at this point I'm not sure exactly which parts need fixed, one or both.

    /**
     * @return Target
     */
    public function getTargetModel()
    {
        if (!$this->_target) {
            $this->_target = (new CustomPagesService())->getTargetById($this->getTargetId(), $this->getPageType(), $this->content->container);
        }

        return $this->_target;
    }

Content Model

This method seems partly correct in that it searches if the method exists but that is as far as I see it going, there are no checks for possible null returns which is slightly understandable and should be done within the module's getUrl().

    /**
     * Returns the url of this content.
     *
     * By default is returns the url of the wall entry.
     *
     * Optionally it's possible to create an own getUrl method in the underlying
     * HActiveRecordContent (e.g. Post) to overwrite this behavior.
     * e.g. in case there is no wall entry available for this content.
     *
     * @param boolean $scheme
     * @return string the URL
     * @since 0.11.1
     */
    public function getUrl($scheme = false)
    {
        try {
            if (method_exists($this->getPolymorphicRelation(), 'getUrl')) {
                return $this->getPolymorphicRelation()->getUrl($scheme);
            }
        } catch (IntegrityException $e) {
            Yii::error($e->getMessage(), 'content');
        }

        return Url::toRoute(['/content/perma', 'id' => $this->id], $scheme);
    }

[luke-]

@ArchBlood Can you provide steps to reproduce this error?

[ArchBlood]

@ArchBlood Can you provide steps to reproduce this error?

Install the latest versions of the Rest API module and make the changes in humhub/legal@eff2bd5 then install the latest version of the Custom Page module and have HTML or other page type and try downloading your user-data from the new "Download My Data".

[luke-]

@ArchBlood Hmm, I've did the steps and was able to download my data.

However, the download on my test installation took a quite long time ( with some data.

We absolutely have to outsource this to an ActiveJob to avoid server overloads and timeouts.

[ArchBlood]

@ArchBlood Hmm, I've did the steps and was able to download my data.

However, the download on my test installation took a quite long time ( with some data.

We absolutely have to outsource this to an ActiveJob to avoid server overloads and timeouts.

In this case I'd have to ask for some assistance when it comes to implementing via ActiveJob, as I've never required this method before, if the error isn't being reported then I can go ahead and close this P/R.

[luke-]

Can you please post the full stacktrace of the error?

[ArchBlood]

Can you please post the full stacktrace of the error?

You can find the full error stack here humhub/legal#68 (comment)

In short we call ContentDefinitions::getContent() from the Rest API module which calls self::getContentMetadata() that calls $content->getUrl(true) which is returning a null object from the Custom Pages module getContentUrl() which is called by getUrl() within the Custom Pages module model.

[ArchBlood]

The only other thing that I can think of is possible issue when testing against the core of master branch for changes made for v1.15.4?

[luke-]

I've created a bug report here: humhub/custom-pages#323