IBX-10854 not possible to hide content draft

[vidarl]

🎫 Issue	IBX-10854

It is not possible to publish content in hidden state

For QA:

Documentation:

[Steveb-p]

If this is true, then a dedicated ContentInfo method should probably be introduced.

[vidarl]

@Steveb-p
Okay, so I first thought about adding function like this:

+    public function isInitialDraft(): bool
+    {
+        return $this->status === self::STATUS_DRAFT && $this->currentVersionNo === 1;
+    }

However, then I realized that this is not needed for ContentInfo as existing ContentInfo::isDraft() will do just fine. This method will only return true if Content in question has never been published. Once Content is published, ContentInfo::isDraft() will still return false when you create new drafts for that content.

Fixed in acbb2d8

[alongosz]

AFAIR this is by-design. There's no purpose to hiding content drafts because content drafts can be read by people with versionread policy, the same as hidden content can be accessed. Are you trying to fix some fatal error or enable hiding drafts? TL;DR;.

[vidarl]

There's no purpose to hiding content drafts because

yes, it is. If you don't want content to be visible at time of publishing.

Are you trying to fix some fatal error or enable hiding drafts?

@alongosz
I am trying to publish content as hidden. The use-case is:
Write a new article but translate it to multiple languages before making it visible. Since admin-ui cannot edit multiple languages for the same version, publish it as hidden would do the trick

edited....

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[vidarl]

ping @alongosz @Steveb-p : I have answered on your comments. Please have a look again

[Steveb-p]

Doesn't this mean, with this change, that for drafts every user has ability to hide & reveal?

Because we won't be checking permission resolver for drafts.

[vidarl]

You are right. Fixed in ca2b306.
Unsure if it 100% safe to have $targets=[] in parameter to canUser() though, but seems to work fine for me when I test with limitation on content/hide policy

@Steveb-p

[konradoboza]

@vidarl please make sure #667 (comment) is addressed, PR is rebased and the CI lits green.

[sonarqubecloud]

Quality Gate failed

Failed conditions
3.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[vidarl]

@vidarl please make sure #667 (comment) is addressed, PR is rebased and the CI lits green.

@konradoboza
Done. Only sonar complains now about duplicated code in Tests. Which should be considered as a false-positive

[konradoboza]

Suggested change

	// If ContentInfo is in draft state, mainocationId is yet not set
	// If ContentInfo is in draft state, mainLocationId is yet not set

[konradoboza]

Suggested change

	self::assertNotNull($publishedContent->contentInfo->getMainLocationId(), 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$publishedContent->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);

[konradoboza]

Is there ->getContentInfo()? If it is we should adjust usages.

Suggested change

	$content = $this->contentService->loadContent($publishedContent->contentInfo->id);
	$content = $this->contentService->loadContent($publishedContent->contentInfo->getId());

[konradoboza]

Suggested change

	self::assertTrue($content->contentInfo->isHidden, 'Content is not hidden');
	self::assertTrue($content->contentInfo->isHidden(), 'Content is not hidden');

[konradoboza]

Suggested change

	self::assertNotNull($publishedContent->contentInfo->getMainLocationId(), 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$publishedContent->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);

[konradoboza]

Suggested change

	self::assertNotNull($parentContent->contentInfo->getMainLocationId(), 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$parentContent->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);

[konradoboza]

Suggested change

	self::assertNotNull($directChildContent->contentInfo->getMainLocationId(), 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$directChildContent->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);

[konradoboza]

Suggested change

	$directChildLocation = $this->locationService->loadLocation($directChildContent->contentInfo->getMainLocationId());
	$directChildLocation = $this->locationService->loadLocation(
	$directChildContent->contentInfo->getMainLocationId()
	);

[konradoboza]

Suggested change

	self::assertNotNull($childContent->contentInfo->mainLocationId, 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$childContent->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);

[konradoboza]

Suggested change

	self::assertNotNull($content->contentInfo->getMainLocationId(), 'Expected mainLocationId to be set for this test case.');
	self::assertNotNull(
	$content->contentInfo->getMainLocationId(),
	'Expected mainLocationId to be set for this test case.'
	);