Follow up for #1896268-16: Add Views integration for translation_entity
Problem/Motivation
function getTranslatorPermissions() should be probably protected
Proposed resolution
make getTranslatorPermissions() protected, parent method, and fixing that in 4 classes
Remaining tasks
- TBD
User interface changes
No UI changes.
API changes
Change to protected.
Original report @dawehner
Follow up for #1896268-16: Add Views integration for translation_entity (and comment 14)
+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/Tests/Views/TranslationLinkTest.phpundefined
@@ -0,0 +1,67 @@
+ function getTranslatorPermissions() {
should be probably protected.
@tim.plunkett
Yes, it 100% should be protected, but the parent method is not, and fixing that in 4 classes is out of scope :(
Comment | File | Size | Author |
---|---|---|---|
#12 | getTranslatorPermissions_protected-1897770-11.patch | 3.51 KB | YesCT |
#10 | getTranslatorPermissions_protected-1897770-10.patch | 4.35 KB | YesCT |
#4 | getTranslatorPermissions_protected-1897770-4.patch | 5.08 KB | jayboodhun |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedoops forgot to update the title
Comment #2
tim.plunkettAnywhere that
function getTranslatorPermissions
appears in code, it should beprotected function getTranslatorPermissions
.This is because it is only called internally, it shouldn't be public facing.
Comment #3
YesCT CreditAttribution: YesCT commentedthanks tim!
I think that's complete info, so it's ready for someone to pick this up and make an initial patch.
Comment #4
jayboodhun CreditAttribution: jayboodhun commentedI have done a search and replace and made sure all these functions are protected
Comment #5
plachLooks good, thanks.
Comment #6
tim.plunkettIt does look good, but it's a follow-up for #1896268: Add Views integration for translation_entity, which isn't in yet, so it will need to be rerolled for that.
Comment #7
plach#4: getTranslatorPermissions_protected-1897770-4.patch queued for re-testing.
Comment #8
YesCT CreditAttribution: YesCT commentedwe got the patch back in #4
Comment #10
YesCT CreditAttribution: YesCT commenteddoes not apply. instead of rerolling, just redid the substitutions.
not my most elegant of command lines (sed bit from http://stackoverflow.com/questions/1895819/find-and-replace-in-multiple-... )
grep -R getTranslatorPermissions * | grep -v patch | cut -d":" -f1 | sort -u | xargs sed -i .sedbak 's/function getTranslatorPermissions/protected function getTranslatorPermissions/g'
find . -name "*.sedbak" -exec rm {} \;
git diff
git diff > getTranslatorPermissions_protected-1897770-10.patch
Comment #12
YesCT CreditAttribution: YesCT commentedoops one too many (one was already protected)
Comment #13
plachThanks
Comment #14
catchCommitted/pushed to 8.x, thanks!
Comment #15.0
(not verified) CreditAttribution: commentedUpdated issue summary since the info was added by tim.