There is currently only one assignable user permission for i8ns Translation module: "Translate nodes". Giving a user this permission makes the "Translation" tab appear for all translatable nodes. I would like to request the addition of a "Translate own nodes" permission which causes the "Translation" tab to show up only for those nodes owned by the user.
Comment | File | Size | Author |
---|---|---|---|
#94 | error.png | 80.52 KB | lomasr |
#81 | translations.jpg | 45.82 KB | mlecha |
#68 | translate-own-nodes-253157-68.patch | 13.71 KB | maxtorete |
#65 | translate-own-nodes-253157-65.patch | 13.71 KB | maxtorete |
#63 | translate-own-nodes-253157-63.patch | 14.13 KB | maxtorete |
Comments
Comment #1
5ven CreditAttribution: 5ven commentedIn Drupal 6 it was easy, although I needed to change the core files. Here's the patch for it. There will "translate own content" - access right item at the admin side.
Comment #2
nedjoMoving to correct project, Drupal core.
Comment #3
j.somers CreditAttribution: j.somers commentedAttached is a D7 implementation of the patch from comment #1.
Comment #4
cburschkaLooks good, but we may need a unit test case for this permission...
Comment #5
Dries CreditAttribution: Dries commentedAgreed with #4. Let's make sure we add some tests for this, and related functionality. Thanks!
Comment #6
j.somers CreditAttribution: j.somers commentedAttached is a simple addition to the existing translation test. A new user is created and an attempt is made to translate a node which has not been created by that user.
Please let me know if a bigger test needs to be created or something else has to be added.
Comment #7
catchWe should probably check for a 200 when they translate a node they do have permission to translate as well. Otherwise looks good.
Comment #8
sun.core CreditAttribution: sun.core commentedComment #9
spacereactor CreditAttribution: spacereactor commentedIf this is push to drupal 8 than is another 2 more years of waiting time. If each user wish to translate their own content will work better for a community, by restricting a site to a small group of trustworthy users to translate the whole community site is not very effective. Sorry i not a programmer but do hope that is can resolve. Thk!
Comment #10
FiNeX CreditAttribution: FiNeX commentedWouldn't be possible to apply the patch on http://drupal.org/node/253157#comment-1153121 in D6 ??????
Comment #11
FiNeX CreditAttribution: FiNeX commentedthis patch is wrong, dont' use it
Comment #12
FiNeX CreditAttribution: FiNeX commentedthis patch is wrong, dont' use it
Comment #13
spacereactor CreditAttribution: spacereactor commentedI use #11 and global content translate premission is broken so all admin can't translate like what FiNeX say but using #12 got another problem, those with translate permission all turn user1 account when they try to translate node. Can someone confirm this or maybe i mess up the patch. Thank.
Comment #14
FiNeX CreditAttribution: FiNeX commented@Spacereactor: what do you exactly mean with "all turn user1 account" ?
Comment #15
spacereactor CreditAttribution: spacereactor commentedI using manually patching your #11 and after that reset my user permission for content translate and own translate. when i user a normal account to login example user20, when i click translate node this user20 turn to user1.
Comment #16
FiNeX CreditAttribution: FiNeX commentedProbably you have some other issues on your setup: I've just tested the patch on a clean drupal installation and when I click on "translate node" the user doesn't change.
Moreover if you look at the source code of the patch you can see that it is just a check on the access control.
Comment #17
spacereactor CreditAttribution: spacereactor commentedI remove all other modules expect drupal 6.16, i18n, token, and views.
I have 4 roles, anonymous user, authenticated user, admin, contributor
Admin is set all user permissions to yes and Contributor is able to create node, edit own node, delete own node and translate own content.
I have 3 users,
user1=myself(super user)
user2=admin role
user3=contributor
After i create a new node with super user account(user1), logout from user1 and login as user3, see the node and can't translate, logout from user3 and login as user2 with admin role, user2 see the node and translate button, once click on the translate button, user2 automatic switch to user1. (something is not right here)
I use user3 with contributor role, create a new node, user1 and user2 can translate that own, but user3 can't translate own node. And when user2 try to translate the user3 node, it change to user1 when click on translate button.
Maybe i didn't apply the patch correct but this is what is getting from manual add patch #11
Comment #18
spacereactor CreditAttribution: spacereactor commentedi zip the translation.module and you can check if i mess up the manually patch.
Comment #19
greenc CreditAttribution: greenc commented+1
Comment #20
FiNeX CreditAttribution: FiNeX commentedI've done some extra testing and I've had your same problem... I'm trying to find the problem. Thanks
Comment #21
FiNeX CreditAttribution: FiNeX commentedPlease, don't use the patch I've uploaded for D6: it doesn't work and has a critical security bug.
You can try using the first patch uploaded here: http://drupal.org/node/253157#comment-1153121
P.s: why the version of this bug has been set to D8 ???
Comment #22
dokumori CreditAttribution: dokumori commented@FiNeX - your comments with the patches have been removed from this issue.
dokumori
Security Team coordinator
Comment #23
FiNeX CreditAttribution: FiNeX commentedThanks @dokumori :-)
Comment #24
minff CreditAttribution: minff commentedI have found that all you have to do to remove translation tabs (and translation rights) from nodes that are not assigned to people (i.e. that are not theirs to edit) is to change the function _translation_tab_access in translation.module, adding there node_access control for updating. It should look like this:
This works well with Node Access User Reference.
PS. This goes for Drupal 6.19 + Internationalization 6.x-1.7
Comment #25
nyleve101 CreditAttribution: nyleve101 commentedJust a quick note for anyone else looking for this that it's been turned into a mini module http://drupal.org/project/translation_own_nodes.
Comment #26
plachComment #27
Maune CreditAttribution: Maune commented#1: translate-own-nodes.patch queued for re-testing.
Comment #29
Gábor HojtsyWhile introducing this feature, it would be important to qualify the other permission. Like "Article: edit own content" / "Article: edit all content" etc. are structured. So at the same time sounds like we should rename "translate content" to "translate all content" to conform to this structure, qualifying that "translate own content" is in itself enough to grant, and there is no need to grant "translate content" (especially since it is wider).
Putting on D8MI sprint as a Novice issue to help people pick it up.
Comment #30
plachWe should also keep in mind that we might need to extend these permissions to any entity type.
Comment #31
Gábor Hojtsy@plach: Well I guess Drupal core should rather have edit/create content permissions per node types which combined with translate permissions would let you create translations, no? A little gotcha there is of course that field translations are really node edits while node translations are node creations (even though we still plan to remove this later method).
Comment #32
fastangel CreditAttribution: fastangel commentedHi guys,
I attach a new patch with change of comment #1 and renamed the permission (I changed all call to this permissions)
Regards.
Comment #33
Gábor Hojtsy@fastangel: Looks pretty good, thanks for picking this up! Sounds like what we need is:
- upgrade path for permission rename
- new tests for the new permission to verify that granting that only grants translation perms to your own content
Comment #34
Gábor HojtsyTagging up.
Comment #35
jibranDon't know how to write a test.
Comment #36
fastangel CreditAttribution: fastangel commentedI have some questions:
Comment #37
Gábor HojtsyWe in fact need two kinds of tests:
- upgrade tests (these are under system module, have language tests there for upgrade path related stuff)
- functionality test (for setting users with this permission and checking access to various nodes)
The second type of test can indeed go to the existing translation test case, if that is already for testing the "translate content" permission :) You'll find existing language upgrade tests with the system module, the new upgrade tests should go there. I think this got complex enough to loose the Novice tag (unfortunately).
Comment #38
penyaskitoRerolled patch from #35. Translation tests passes, but cannot launch all tests and QA is down temporally, so I don't set to "Needs review".
Comment #39
penyaskitoQA is enabled again, let's check the reroll was fine.
Comment #40
maxtorete CreditAttribution: maxtorete commentedHi,
Here at Penyaskito home are 3:30am and we are closing tonight sprint, I upload this WIP patch for functionality test. Tomorrow morning I'll finish it.
Greets!
Comment #41
sxnc CreditAttribution: sxnc commentedJust tested this locally and it worked fine for me with a user having only the permission to edit his own nodes.
Comment #42
maxtorete CreditAttribution: maxtorete commentedI have added a new assertion to check that a user with "Translate own content" role can't get the create translation form via URL, and it fails because this condition isn't check, so the user can create a translation of other user's content.
For example, on a multilingual site (en, es) a user X creates a node with nid 23 on english language. Other user Y with "Translate own content" role opens node 23 and can't see translate link (so it's working fine), but if she types on her browser the url http://mysite/node/add/page?translation=23&target=es she (Y user) must get a 403 page, but instead she gets the translate form and can send a translation for other users node.
Because translations won't become new nodes anytime soon, should we just ignore this, or an action should be taken?
Comment #43
Gábor Hojtsy@Maxtorete: hm, if we want to have this permission enforced, then we should not leave holes like that :) When this is ported to entity translations (instead of creating new nodes), we will port this there, but we need to fix it here now. Let's fix that too.
Comment #44
maxtorete CreditAttribution: maxtorete commentedOk, I'm working on it now ;-)
I can't find language upgrade test on system module, where are them?
Comment #45
Gábor HojtsyThe upgrade test database dumps are in core/modules/system/tests/upgrade/, the actual tests are in core/modules/system/lib/Drupal/system/Tests/Upgrade/LanguageUpgradePathTest.php
Comment #46
maxtorete CreditAttribution: maxtorete commentedI have fixed the issue pointed on comment 42.
Now I have another doubt, If a user has the "Translate all content" but not the "Create new content", he can't translate content. I think this could be wrong, because, for example, you want to give access to a professional translator to translate current content but not to create new.
Comment #47
Gábor HojtsyI think THAT is fine though, or at least definitely out of scope for this patch. Also, since we are targeting to remove node copy as translations, people will not need the create content permission to add translations eventually hopefully.
Comment #48
maxtorete CreditAttribution: maxtorete commentedOk thanks, waiting then to patch test & review.
Comment #49
Gábor HojtsyExtra whitespace on the empty line. Should have no spaces.
You are in fact renaming the "translate content" permission :)
"To translate all content" in the help text sounds like a search and replace issue, should just be "translate content" AFAIS.
Should update the explanation, the code is not just checking the translate all content permission anymore, its more complex.
Can we put this kind of code in a hook_node_access() somehow? It looks like being at an dd place in the middle of a translation form alter to throw the user out like we do. From a node access hook, we can just check the create op and if a translation is being created somehow. That sounds like could hold its own problems, but at least it would be in the access checking flow :)
Comment #50
maxtorete CreditAttribution: maxtorete commentedI tried it but I can't get a way to load node details on hook_node_access, I will look for it again :-)
Comment #51
maxtorete CreditAttribution: maxtorete commentedOk, now I've got the problem, "translation_node_access" is being called more than once on the same request, only has to get the correct one.
Comment #52
maxtorete CreditAttribution: maxtorete commentedI don't get any way to use hook_node_access to control permissions, because on create we only get the node type on $node.
Comment #53
maxtorete CreditAttribution: maxtorete commentedI have found the bug, it is on translation_node_prepare, no need to implements hook_node_access :-). I will fix later!
Comment #54
maxtorete CreditAttribution: maxtorete commentedAnyway, I'm going to implement hook_node_access, because if it isn't implemented the user gets a blank create node form and could send it creating a new node (thought not a translation).
Comment #55
maxtorete CreditAttribution: maxtorete commentedNew patch attached. I have moved permission check from hook_node_prepare to hook_node_access and create a new function to check if a user can translate a node.
Comment #56
penyaskitoWe don't need to wrap test messages in t(). See http://drupal.org/simpletest-tutorial-drupal7#t. But the way the rest of the tests in the same file use t() on messages. Maybe we can leave this, but create a follow-up issue for cleaning this.
Use 403 instead of '403'. There are several cases.
I'm not sure if we should use an auxiliar function. In any case, it should be prefixed with an _, I don't think we want to expose this as API.
AFAIK, Drupal core standards say:
Comment #57
maxtorete CreditAttribution: maxtorete commentedNew patch uploaded with upgrade test!
Ok, I will create the new issue.
Done.
That permission is checked more than once so I think it could help on future changes on translation system if we left it on an auxiliar function.
Also I don't know if we should hide the function, it could be used by other functions or modules to check if a user can translate a node. Maybe we could refactor some functions on location.module to check user translation permissions, for example when listing links to translate actions, so once the translation system is migrated from the current node copy system we only has to modify that function.
Comment #58
pp CreditAttribution: pp commentedWhy not contains the help text information about "translate own content" permission?
Is "global $user" necessary?
Tests run well.
Comment #59
maxtorete CreditAttribution: maxtorete commentedThis text come from a previous patch, I think it's search & replace issue, fixed.
No it isn't, after implementing hook_node_access the user object isn't used anymore on that function. I forgot to delete that line, thanks for pointing it.
New patch with these corrections added.
Comment #60
maxtorete CreditAttribution: maxtorete commentedComment #62
maxtorete CreditAttribution: maxtorete commentedI forgot to add module.install in last patch... :-(
Comment #63
maxtorete CreditAttribution: maxtorete commentedIgnore last patch, this is the correct one.
Comment #64
fastangel CreditAttribution: fastangel commentedOne comment about this function: translation_user_can_translate_node($node, $account). I think that the best form is translation_user_can_translate_node($node, $account = NULL) if $account is null then use a user logged on the web. With this I think you can save declare global $user. (I think this function will be usually called with the default user). What do you think?
Regards.
Comment #65
maxtorete CreditAttribution: maxtorete commentedI think you are right, patch attached with that change.
Comment #66
mlecha CreditAttribution: mlecha commentedJust tried the #63 patch in D7, which worked. The patch has the functionality I need. Is this the sort of thing that gets backported to D7? If not, could it be ported to a module that overrides the core functions?
Thanks for your patience...
Comment #68
maxtorete CreditAttribution: maxtorete commentedFixed warning from comment 65 patch.
Comment #69
pp CreditAttribution: pp commentedI think this patch(68) is good for commit.
Comment #70
Bojhan CreditAttribution: Bojhan commentedEhh, we dont allow the word "nodes". This should be "Translate own content" see http://drupal.org/node/604342.
Or is the issue title wrong? then never mind my comment.
Comment #71
Gábor HojtsyComment #72
Bojhan CreditAttribution: Bojhan commentedHeh, sorry - back to RTBC.
Comment #73
penyaskito#68: translate-own-nodes-253157-68.patch queued for re-testing.
Comment #74
Dries CreditAttribution: Dries commentedThis looks good to me. Committed to 8.x. Thanks all.
I'd like to see us update CHANGELOG.txt with this and other recent D8MI additions.
Comment #75
Gábor HojtsyThanks Dries. We will get to submitting CHANGELOG.txt patches and change notices soon. So far the important focus was to get stuff committed before they become stale and people wander off :)
Comment #76
Gábor HojtsyAdded http://drupal.org/node/1776752 as a change notice for this. CHANGELOG.txt patch forthcoming from rvilar.
Comment #77
Gábor HojtsyAdd missing language-content tag.
Comment #78
Gábor HojtsyChangelog action happening in #1777870: Catch up changelog.txt with recent languages changes, removing off sprint. Thanks all!
Comment #80
rameshbabu.g CreditAttribution: rameshbabu.g commentedHi Somers,
Thank you for your patch . It works for me.
By using this I got one more permission on translation i.e 'Translate own content' .It is fine
But here one more complication is there .I need to restrict 'Translate own content ' permission to particular content type .
Can you/anyone please suggest me to reach this ...(I m using drupal 7)
Thanks in advance.
Comment #81
mlecha CreditAttribution: mlecha commentedI'm testing this both in Drupal 8.x-dev and the patch at #1 above in D7.
Not sure if this is a bug, or if my use case is an anomaly?
We're a non-profit selling job postings on a job board. After the user has paid for and posted their job, access to "Create new content" is revoked. But they still need access to translate their own posting for free.
These users have permission to "Translate own content" but not to "Create new content".
In that scenario the translation tab shows up, but not the link (or permission) to "Add a translation".
Any advice would be much appreciated. Easy fix?
Or a bounty could be offered for a custom fix.
Comment #82
mlecha CreditAttribution: mlecha commentedFound that I could add a condition in translation.pages.inc to make the add translation link appear:
if ((node_access('create', $node)) || (($node->uid == $user->uid) && user_access('translate own content'))) {
But that link only leads to an "Access denied" when the node is attempting to be created. Looking for where in core that permission is checked?
Comment #83
Gábor HojtsyTranslation is a node crreation, so the user needs that permission too. If you want to prevent them from submitting further untranslated content, you would need to have write some custom code to catch that situation on node creation, if it is not created as a translation.
Comment #84
bisonbleu CreditAttribution: bisonbleu commentedThe translation_own_nodes module (#25) seems to be broken with drupal 6.26. The simple patch in #24 works.
Comment #85
thegreatone CreditAttribution: thegreatone commented68: translate-own-nodes-253157-68.patch queued for re-testing.
Comment #87
thegreatone CreditAttribution: thegreatone commentedwich one should be used for D7? it's confusing..it started as D7 then switched to D8...
Entity translation...
Comment #88
bforchhammer CreditAttribution: bforchhammer commentedPatch has been committed already(#74), setting back to fixed.
@thegreatone: this is against D8 core, I don't think there's an equivalent D7 version of this patch.
Comment #89
Gábor HojtsyThe D7 issue for this is at #1829630: Improve workflow permissions (match D8 solution). It is in a contributed module, so its not this same issue reused. As pointed out above there is also a mini module at https://drupal.org/project/translation_own_nodes that implements some of this at least.
Comment #90
joshua.boltz CreditAttribution: joshua.boltz commentedAnyone know if there is a custom solution that handles mlecha's issue. Or any ideas on how to hook into Drupal?
I need a user to be able to create a translation of a node but not be able to create any new node.
Comment #91
pieterdt CreditAttribution: pieterdt commentedreopening, as this seems to be be gone from D8.1?
Comment #92
mallezieThis indeed looks gone to me as well.
Comment #93
RobertoGuzman CreditAttribution: RobertoGuzman commenteddoes these patches apply to Drupal 8.2.1 ?
Comment #94
lomasr CreditAttribution: lomasr at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedTried to apply patch in #63 on drupal 8.3.x. But it gives me 'No such file or directory' error . Please see the screenshot. Is it applicable for 8.3.x ?
Comment #102
bahuma20Here a little food for thought:
Is it possible to handle this via an additional operation in hook_node_access_records (grant_translation)? So that we could handle a scenario where a user should only be able to translate specific nodes, but not edit them.
Comment #103
s-jack CreditAttribution: s-jack commentedSorry about my post, as not a developer.
Is this issue a low priority?
Spend a long long time.
The ability to restrict others from translating your posts is important.
I'm eager for progress.
Thank you for the development.
Comment #105
weynhamzI like this idea, I am working on a project in which the access is set on a per-node basis, while translation form ignores the entity_access checks and there is no way to add the access check for translation, which causes a permission bypass.
Comment #108
BerdirI'm not sure what exactly happened here either, but #2972308: Allow users to translate content they can edit finally happened and I think covers this and more use cases. Closing, not sure what status to use exactly.