-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added support for translation of News-Extension records with SUMM AI model #1
base: 12.4
Are you sure you want to change the base?
Conversation
…model. ContentAiPageProvider and AiTranslationPageService were renamed from Page to Content as those classes support both pages and news records.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @coderanian, I created a PR for easier review of your feature. So far, it looks very good. I just found some minor tweaks which I kindly ask you to address. You can push your changes in the same branch and I'll recheck it then. Once everything is looking good, we can finally make a PR to the upstream repository.
use DMK\MkContentAi\Domain\Model\News; | ||
use DMK\MkContentAi\Domain\Repository\NewsRepository; | ||
use DMK\MkContentAi\Http\Client\SummAiClient; | ||
use function _PHPStan_a3459023a\RingCentral\Psr7\str; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this class import
switch ($this->typo3Version->getMajorVersion()) { | ||
case 11: | ||
$translateContentPlainAction = ' | ||
<a href='.$uriGenerated.' class="btn btn-default" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href='.$uriGenerated.' class="btn btn-default" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default"> | |
<a href="'.$uriGenerated.'" class="btn btn-default" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default"> |
case 12: | ||
$iconFactory = GeneralUtility::makeInstance(IconFactory::class); | ||
$translateContentPlainAction = ' | ||
<a href='.$uriGenerated.' class="dropdown-item dropdown-item-spaced" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default icon-actions-translate"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href='.$uriGenerated.' class="dropdown-item dropdown-item-spaced" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default icon-actions-translate"> | |
<a href="'.$uriGenerated.'" class="dropdown-item dropdown-item-spaced" title="'.$labelActionName.'"><span class="t3js-icon icon icon-size-small icon-state-default icon-actions-translate"> |
use TYPO3\CMS\Core\Information\Typo3Version; | ||
use TYPO3\CMS\Core\Utility\GeneralUtility; | ||
use TYPO3\CMS\Extbase\Utility\LocalizationUtility; | ||
use TYPO3\CMS\Recordlist\Event\ModifyRecordListRecordActionsEvent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add typo3/cms-extbase
and typo3/cms-recordlist
dependencies to composer.json
.
class NewsRecordEventListener | ||
{ | ||
protected ContentAiTranslationProvider $contentAiTranslationProvider; | ||
private Typo3Version $typo3Version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be protected
as well
@@ -27,6 +27,13 @@ services: | |||
method: handleEvent | |||
event: TYPO3\CMS\Filelist\Event\ProcessFileListActionsEvent | |||
|
|||
DMK\MkContentAi\Backend\EventListener\NewsRecordEventListener: | |||
public: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is superfluous, event listeners are public by default (see https://github.com/TYPO3/typo3/blob/v12.4.24/typo3/sysext/core/Classes/DependencyInjection/ListenerProviderPass.php#L81).
tags: | ||
- name: event.listener | ||
method: modifyRecordListActions | ||
event: TYPO3\CMS\Recordlist\Event\ModifyRecordListRecordActionsEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be omitted, because event is auto-discovered (see https://github.com/TYPO3/typo3/blob/v11.5.41/typo3/sysext/core/Classes/DependencyInjection/ListenerProviderPass.php#L118).
[ | ||
'tx_mkcontentai_original_news_uid' => [ | ||
'config' => [ | ||
'type' => 'passthrough', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for passthrough
here? IMHO it should be group
.
|
||
During translation new News record is created with translated title, teaser and bodytext. Created record must be enabled manually. New added key in tx_news_domain_model_news `tx_mkcontentai_original_news_uid` can then be used to link original news record (e.g. for switch between original and simplified language). | ||
|
||
![](Documentation/Images/SummAiAPI/summ-ai-news-example.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an alt text to the image.
<f:for each="{settingsRequestDTO.newsContentTypes}" as="selectedCType"> | ||
{selectedCType}, | ||
</f:for> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<f:for each="{settingsRequestDTO.newsContentTypes}" as="selectedCType"> | |
{selectedCType}, | |
</f:for> | |
<f:for each="{settingsRequestDTO.newsContentTypes}" as="selectedCType" iteration="iterator"> | |
{selectedCType}{f:if(condition: iterator.isLast, else: ',')} | |
</f:for> |
ContentAiPageProvider and AiTranslationPageService were renamed from Page to Content as those classes support both pages and news records.