Skip to content
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

IBX-4452: Added JMSTranslation configuration and missing translations #184

Merged
merged 1 commit into from
Dec 22, 2022
Merged

IBX-4452: Added JMSTranslation configuration and missing translations #184

merged 1 commit into from
Dec 22, 2022

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Dec 22, 2022 •
edited
Loading

Question Answer
JIRA issue IBX-4452
Type improvement
Target Ibexa version v4.4
BC breaks no

This PR adds:

  • JMS Translation configuration into core bundle.
  • Missing TranslationDefinitionProvider class.
  • Provided missing Translation definitions in .xliff.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

@kisztof kisztof requested a review from a team December 22, 2022 08:35
@kisztof kisztof marked this pull request as ready for review December 22, 2022 08:47
Steveb-p
Steveb-p approved these changes Dec 22, 2022
View reviewed changes
...dle/Core/DependencyInjection/Security/PolicyProvider/PolicyTranslationDefinitionProvider.php Outdated Show resolved Hide resolved
...dle/Core/DependencyInjection/Security/PolicyProvider/PolicyTranslationDefinitionProvider.php Outdated Show resolved Hide resolved
...dle/Core/DependencyInjection/Security/PolicyProvider/PolicyTranslationDefinitionProvider.php Outdated
Comment on lines 20 to 38
(new Message('role.policy.content.all_functions', 'forms'))->setDesc('Content / All functions'),
(new Message('role.policy.content.cleantrash', 'forms'))->setDesc('Content / Cleantrash'),
(new Message('role.policy.content.create', 'forms'))->setDesc('Content / Create'),
(new Message('role.policy.content.diff', 'forms'))->setDesc('Content / Diff'),
(new Message('role.policy.content.edit', 'forms'))->setDesc('Content / Edit'),
(new Message('role.policy.content.hide', 'forms'))->setDesc('Content / Hide'),
(new Message('role.policy.content.manage_locations', 'forms'))->setDesc('Content / Manage locations'),
(new Message('role.policy.content.pendinglist', 'forms'))->setDesc('Content / Pendinglist'),
(new Message('role.policy.content.publish', 'forms'))->setDesc('Content / Publish'),
(new Message('role.policy.content.read', 'forms'))->setDesc('Content / Read'),
(new Message('role.policy.content.remove', 'forms'))->setDesc('Content / Remove'),
(new Message('role.policy.content.restore', 'forms'))->setDesc('Content / Restore'),
(new Message('role.policy.content.reverserelatedlist', 'forms'))->setDesc('Content / Reverserelatedlist'),
(new Message('role.policy.content.translate', 'forms'))->setDesc('Content / Translate'),
(new Message('role.policy.content.translations', 'forms'))->setDesc('Content / Translations'),
(new Message('role.policy.content.unlock', 'forms'))->setDesc('Content / Unlock'),
(new Message('role.policy.content.urltranslator', 'forms'))->setDesc('Content / Urltranslator'),
(new Message('role.policy.content.versionread', 'forms'))->setDesc('Content / Versionread'),
(new Message('role.policy.content.versionremove', 'forms'))->setDesc('Content / Versionremove'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I'd consider pulling the common parts of those strings into variables, injected into these Messages using sprintf or straight up string interpolation.

But it depends on whether the readability would suffer, and if others agree.

@konradoboza konradoboza requested a review from a team December 22, 2022 08:57
@kisztof kisztof force-pushed the ibx-4452-deprecate-translation-extractors branch from 8db4d5d to daa308b Compare December 22, 2022 09:21
@konradoboza konradoboza requested a review from a team December 22, 2022 09:22
@kisztof kisztof force-pushed the ibx-4452-deprecate-translation-extractors branch 2 times, most recently from fafde70 to 1b11165 Compare December 22, 2022 09:32
@kisztof kisztof force-pushed the ibx-4452-deprecate-translation-extractors branch 2 times, most recently from 6e26b3e to 0d25658 Compare December 22, 2022 12:01
@kisztof kisztof force-pushed the ibx-4452-deprecate-translation-extractors branch from 0d25658 to 4b3e340 Compare December 22, 2022 12:57
@sonarcloud
Copy link

sonarcloud bot commented Dec 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 13 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alongosz alongosz merged commit 117209c into main Dec 22, 2022
@alongosz alongosz deleted the ibx-4452-deprecate-translation-extractors branch December 22, 2022 13:59
@alongosz alongosz restored the ibx-4452-deprecate-translation-extractors branch December 22, 2022 13:59
@alongosz alongosz deleted the ibx-4452-deprecate-translation-extractors branch December 22, 2022 14:00
@alongosz
Copy link
Member

Merged per internal agreement with QA.

@mikadamczyk mikadamczyk mentioned this pull request Jun 29, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Steveb-p Steveb-p Steveb-p approved these changes

@alongosz alongosz alongosz approved these changes

@konradoboza konradoboza konradoboza approved these changes

Assignees

@adamwojs adamwojs

@mikadamczyk mikadamczyk

@Steveb-p Steveb-p

@alongosz alongosz

@ciastektk ciastektk

Labels
Improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kisztof @alongosz @Steveb-p @konradoboza @adamwojs @mikadamczyk @ciastektk