Skip to content

[TASK] Make the plugin a CType #1811

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.

Sign up for GitHub

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

Jump to bottom
Open
linxpinx wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[TASK] Make the plugin a CType #1811

linxpinx wants to merge 2 commits into from

Conversation

@linxpinx
Copy link
Contributor

@linxpinx linxpinx commented Sep 25, 2025 •
edited by oliverklee
Loading

Fixes #1332

@linxpinx linxpinx added the task label Sep 25, 2025
@linxpinx linxpinx mentioned this pull request Sep 25, 2025
Closed
oliverklee
oliverklee requested changes Sep 25, 2025
View reviewed changes
Resources/Private/Language/de.locallang_db.xlf Outdated
</trans-unit>
<trans-unit id="plugin.tea_index.title">
<source>Tea Index</source>
<target>Tea Index</target>
Copy link
Contributor

@oliverklee oliverklee Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English: Tea list
German: Teelist

Copy link
Contributor Author

@linxpinx linxpinx Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Resources/Private/Language/de.locallang.xlf Outdated
<target>Tee-Einzelansicht</target>
</trans-unit>
<trans-unit id="plugin.tea_frontend_editor" approved="yes">
<trans-unit id="plugin.tea_front_end_editor" approved="yes">
Copy link
Contributor

@oliverklee oliverklee Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

@linxpinx linxpinx Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that the label was missing before, but now it seems to be there. I'll change it.

Configuration/TSconfig/Page/ContentElementWizard.tsconfig Outdated
@@ -0,0 +1,16 @@
mod.wizards.newContentElement.wizardItems.ext-tea {
Copy link
Contributor

@oliverklee oliverklee Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move adding the wizard to a separate, later PR.

Copy link
Contributor Author

@linxpinx linxpinx Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i`ll create a task for that. #1830

Classes/Upgrades/ListTypeToCTypeUpdate.php
use TYPO3\CMS\Install\Attribute\UpgradeWizard;

#[UpgradeWizard('teaListTypeToCTypeUpdate')]
final class ListTypeToCTypeUpdate extends AbstractListTypeToCTypeUpdate
Copy link
Contributor

@oliverklee oliverklee Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This upgrade wizard needs functional tests.

Copy link
Contributor

@oliverklee oliverklee Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we need tests for the copied base class. (When we copy code, it becomes our responsibility, and it then falls under the same code quality standards as the rest of our code.)

Copy link
Contributor Author

@linxpinx linxpinx Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliverklee I added some functional tests for the Upgrade Wizard.
As for the copied base class: I don't want to put so much work into a class that I only need for one version. Is there a solution that would suit both you and me? Otherwise, I suggest opening a separate PR for this issue.

Build/phpstan/phpstan.neon
message: 'Use PSR-7 API instead'

excludePaths:
# We need this Class only for V12. So we exclude it here
Copy link
Contributor

@oliverklee oliverklee Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we should allow PHPStan to scan it. It should be possible to run the upgrade wizard in V13 as well.

Copy link
Contributor Author

@linxpinx linxpinx Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliverklee in V13 this Class is in cms-install. So we don't need the Class from Lina. For that reason I decided to exclude it from PHPStan.

Copy link
Contributor

@oliverklee oliverklee Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't exclude the upgrade wizard from PHPStan. Otherwise, if it is not scanned, we cannot see any warnings in the class.

oliverklee
oliverklee requested changes Oct 23, 2025
View reviewed changes
Build/phpstan/phpstan.neon
message: 'Use PSR-7 API instead'

excludePaths:
# We need this Class only for V12. So we exclude it here
Copy link
Contributor

@oliverklee oliverklee Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't exclude the upgrade wizard from PHPStan. Otherwise, if it is not scanned, we cannot see any warnings in the class.

Classes/Upgrades/ListTypeToCTypeUpdate.php
use TYPO3\CMS\Install\Attribute\UpgradeWizard;

#[UpgradeWizard('teaListTypeToCTypeUpdate')]
final class ListTypeToCTypeUpdate extends AbstractListTypeToCTypeUpdate
Copy link
Contributor

@oliverklee oliverklee Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we need tests for the copied base class. (When we copy code, it becomes our responsibility, and it then falls under the same code quality standards as the rest of our code.)

@linxpinx linxpinx force-pushed the task/1332-make-plugin-ctype branch from 23af684 to 14eba5a Compare October 31, 2025 19:10
@oliverklee oliverklee added this to the 4.0.0: Breaking changes milestone Nov 30, 2025
@oliverklee oliverklee moved this to In Progress in Best Practices Remote Day Nov 30, 2025
Starkmann added a commit that referenced this pull request Dec 3, 2025
@Starkmann
[TASK] increase test coverage and also test user rights migration
a5d21e9 
Related:  #1811
@oliverklee oliverklee changed the title [TASK] Make plugin CType [TASK] Make the plugin a CType Dec 4, 2025
@oliverklee oliverklee force-pushed the task/1332-make-plugin-ctype branch from a5d21e9 to 4b130dc Compare December 4, 2025 09:35
@coveralls
Copy link

coveralls commented Dec 4, 2025

Pull Request Test Coverage Report for Build 19924290557

Details

  • 10 of 172 (5.81%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-57.2%) to 42.756%
Changes Missing Coverage Covered Lines Changed/Added Lines %
Classes/Upgrades/AbstractListTypeToCTypeUpdate.php 0 162 0.0%
Totals Coverage Status
Change from base Build 19890254035: -57.2%
Covered Lines: 121
Relevant Lines: 283
💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oliverklee oliverklee oliverklee requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@Starkmann Starkmann

Labels

task

Projects

Best Practices Remote Day
Status: In Progress

Milestone

4.0.0: Drop support for TYPO3 11LTS a...

Development

Successfully merging this pull request may close these issues.

Make the plugin a CType

5 participants

@linxpinx @coveralls @oliverklee @Starkmann