The 2.x version is a significant rewrite to fix security bugs. There should be some tests to ensure that the security bugs stay fixed.

Issue fork private_content-2948622

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

AdamPS created an issue. See original summary.

adamps’s picture

adamps commented
Project: Private » Private content
adamps’s picture

adamps commented
Status: Active » Closed (duplicate)

Fixed in #3074618: Fix tests

adamps’s picture

adamps commented
Status: Closed (duplicate) » Active

Actually this issue is different. The other issue got the existing tests working. This issue is to make better tests for the security bugs that were present in 1.x.

batigolix made their first commit to this issue’s fork.

batigolix opened merge request !4

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Active » Needs review
Issue tags: +finalist-sprint

With the help of AI I expanded the unit tests, which now pass in the Gitlab Pipeline.

Please review

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Priority: Normal » Major
arjenk’s picture

arjenk commented
Status: Needs review » Needs work

Nice to see such a big addition in unittests, some random thoughts;

  • setUp() is public instead of protected in 5 files (PrivateContentTestBase, PrivateUnpublishedTest, PrivateFieldPermissionTest, PrivateComputedFieldTest, PrivateContentTypeTest). PHPUnit/Drupal requires protected.
  • 3 test classes bypass the base class - PrivateUnpublishedTest, PrivateFieldPermissionTest, and PrivateComputedFieldTest extend BrowserTestBase directly and duplicate the exact setUp() from PrivateContentTestBase. They should extend PrivateContentTestBase instead.
  • "// AI generated.", do we need this?
  • For a future follow up, i suggest adding tests for revisions, and the bulk actions (nodeMakePrivate/NodeMakePublic)
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Needs work » Needs review

This feedback has been addressed:

- setUp() is public instead of protected in 5 files (PrivateContentTestBase, PrivateUnpublishedTest, PrivateFieldPermissionTest, PrivateComputedFieldTest, PrivateContentTypeTest). PHPUnit/Drupal requires protected.
- 3 test classes bypass the base class - PrivateUnpublishedTest, PrivateFieldPermissionTest, and PrivateComputedFieldTest extend BrowserTestBase directly and duplicate the exact setUp() from PrivateContentTestBase. They should extend PrivateContentTestBase instead.
- "// AI generated.", do we need this?

Please make a new issue for: "i suggest adding tests for revisions, and the bulk actions (nodeMakePrivate/NodeMakePublic)"

This can be reviewed.

arjenk’s picture

arjenk commented
Status: Needs review » Reviewed & tested by the community

  • batigolix committed bc40df8f on 8.x-2.x 
    Add comprehensive test coverage for private content access control

feat...
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.