Thank you guys for your sharing, we will have a look.

If you are interested by the development itself you can CC yourself in the story.

I had a quick look at both implementation, I think we will be closer to @m-lavrinenko implementation, at least because we don't target private attachments yet.

hehe, perfect example of project planning and coordination :-)

Hello,

Surprisingly, we also somehow close to release this feature. Here is our implementation: https://github.com/Enalean/tuleap/pull/17/files
It follows the requirements (https://tuleap.net/plugins/tracker/?aid=10654#followup_78665) and fully usable, except install/upgrade matters which are not fully tested.

@vaceletm feel free to close PR any time you want to, we've created it only to present the work

as well as @matevy we hope it would be at least some of help

Hi,

if of any value at all, please find our changes which are not entirely complete on https://github.com/matevy/tuleap/tree/12.4-comment-permissions
It is based on 12.4. Both Files and Comments are supported.

Export does not work and API we have not checked yet. But import from Bugzilla using bugzilla's private field works for both Comments and Files.

In order to enable use permissions on comments and filed, one must create tracker field named "comment_file_permissions". It can be of any type, but it looks best if it is of type Checkbox.

It can be placed wherever in the layout, but it looks OK in Access Information section. It would be even better if the field could be hidden, but this is not possible with Tuleap at the moment.

Add value "Enabled" and make it default selection.

Permission of this field are used for all artifact comments and files which have option "Use permissions" selected. In this case each file and comment is checked if it can be displayed to specific user. If that user can READ field named comment_file_permissions, then the user can also view comments and files with "Use permissions" selected.
If user has UPDATE permission to field "comment_file_permissions", he/she can set which files/comment "Use permissions"

We are going to start the implementation in current release.

Hi,

damn, we have just implemented it and today is the day off here so I am cleaning implementation to push for you to criticise it :-)
But yours will probably be better. When do you plan to develop it?

Hi Manuel,

idea with use_comment_permissions is perform permission checking only for those comments which have this feature enabled to not degrade performance for systems not requiring this.

If comment has this enabled, permissions table would be checked additionally. Isn't this the way it is done for artefacts?

Hi all,

I'm a bit concerned about the current path forward.

As we already discussed, permissions with booleans (true/false, public/private) are too limited and doesn't match Tuleap permissions standards (group based). With the current implementation proposable based on a toggle use_comment_permissions, how can it evolve to a group based permission system ?

Hi Maxim,

I have pushed the implementation transferred to Tuleap 12.2 source code.
See last commit at https://github.com/matevy/tuleap

You can see it in action by changing tables in mysql. Create 1 artefact with 3 comments and 3 attachments.
Do this as project admin. Then open the artefact with non-admin member of the project. You will see all 3 comments and all 3 files.

Then in mysql:
select * from tracker_changeset_comment;
+----+--------------+-----------------+--------------------+-----------+--------------+--------------+-----------------+-------------+-------------------------+-------------------------+
| id | changeset_id | comment_type_id | canned_response_id | parent_id | submitted_by | submitted_on | body | body_format | old_artifact_history_id | use_comment_permissions |
+----+--------------+-----------------+--------------------+-----------+--------------+--------------+-----------------+-------------+-------------------------+-------------------------+
| 1 | 2 | NULL | NULL | 0 | 102 | 1606762163 | prvi komentar | text | NULL | 0 |
| 2 | 3 | NULL | NULL | 0 | 102 | 1606762180 | drugi komentar | text | NULL | 0 |
| 3 | 4 | NULL | NULL | 0 | 102 | 1606762204 | tretji komentar | text | NULL | 0 |
+----+--------------+-----------------+--------------------+-----------+--------------+--------------+-----------------+-------------+-------------------------+-------------------------+


And then change permission:

update tracker_changeset_comment SET use_comment_permissions=1 WHERE id=2;


And refresh the web page. Non-admin user should no longer see one comment.

The same goes with file attachments:
update tracker_fileinfo SET use_file_permissions=1 WHERE id=102;

Right now it is made that having use_comment_permissions/use_file_permissions set to 1 already means the comment/file is private. It must be extended with the functionality that in case use_comment_permissions/use_file_permissions is set to 1, code will lookup into permissions table and decide whether current user has access or not.

That is easy to change. More important and problematic is web UI side of things where user must be able to select permissions of the comment being created and change them later. For our use case it should be enough to have just Private/Public, but for the platform it would be better to be more flexible or even configurable how many options there are. For somebody only requiring Private/non-private too many options would be overkill.

ok, in the meantime we will reintegrate this to the latest tuleap version and verify if all works. Will keep you posted

Right, we'll extend that approach and publish some beta in a week or two to discuss it. Right now there is no code, because we are working on another feature

something more...

right now the idea is to check whether permission for comment is enabled or not. Then we verified if this choice is taken into account at all levels/interfaces/modules...

to be able to test we have made the following hard code: if comment permission is enabled, that simply means comment is private. Private means (for now) Admin only. Also for future Admin will always have ability to view comments automatically. But for regular users additional lookup in permissions table can be done to get the info whether that specific user has access to the comment or not. Or group.
I hope we did the foundation right and we wanted it to be possible to extend it and we tried to make it as minimalistic as possible in order to be easier to be integrated into the main source tree.

Hi,

in this current version it has indeed limitation to tracker admin as like I said we did not develop yet GUI portion where one could set up more fine grained rules. The good thing is checking is performed just on one place at this current implementation and then used all around the system. It works with emails sent out, over API, web UI, ... previous variant was leaky.

actually we have the same use-case as you, right now we are solving this by having developers in admin group :-(
in any case I am very interested on cooperating on this feature and make it upstream so everyone else starts using it and maybe improving.
If yours is more advanced, could you share it and maybe we can all use yours? It is always to have just one solution, not 2.

thanks,
Matevz

Have a look at my last commit at https://github.com/matevy/tuleap
It implements permission enforcement for both comments and files attached to the Artefact.

Seems like it have been simplified even more than before. Am I right that it allows access only for tracker admin and original submitter?

It won't work for us, because our use-case is an issue opened by a client and then two or more developers can discuss this issue in private messages.

 

Hi Maxim,

I pushed our changes to github, but unfortunately this is based on Tuleap 11.9, we will merge it to the latest one soon, but I hope there are no huge differences anyway.

Have a look at my last commit at https://github.com/matevy/tuleap

It implements permission enforcement for both comments and files attached to the Artefact.

Ah yes, that was rewritten from scratch. We tried 2 simplified approaches first to learn how it should be done correctly.
Any chance I could create another PR in the evening so you can have a look. I can not do it before.

Hi Matevz,

Sure. Is there any new progress on https://github.com/Enalean/tuleap/pull/11/files ?
As I can see, Manuel said that default permission system shouldn't be used in this feature. That's why I've proposed alternative approach.

Hi Maxim,

we actually implemented full enforcement and storage of comment and attachment permissions the same way it is done elsewhere in Tuleap, but for now only in DB but did not have time to push it yet.
What is missing is only user interface for it, right now we can only set it directly in DB or at import scripts.

Could we join the efforts on this one?

We started to implement it in a bit simplified way:

- new column "private" on tracker_changeset_comment table
- new table tracker_private_comment_permission (id, tracker_id, ugroup_id)
- new administration page "Follow-up permissions" in "Permission" tab at /plugins/tracker/?tracker=X&func=admin with "Allow to read and write private follow-ups for those user groups: " (multi-select of project user groups)
- new method Tracker_Artifact_Changeset_CommentDao::searchLastAllowedVersion(int $changeset_id, int[] $ugroup_ids)
- modification of Tracker_Artifact_Changeset::getComment to use searchLastAllowedVersion instead of searchLastVersion
- modification of follow-up area template to add checkbox "private"
- modification of controller which saves changeset to use new "private" field

This how it works in Redmine from which we want to migrate.

Hi,

I hope this is what you wanted:
https://github.com/Enalean/tuleap/pull/11

Sorry, I forgot about that.

Could you create a PR (against your depo's master for instance) so we can comment within ?

Hi Manuel,

were you able to take a look at the proposed change?

regards,
Matevz

Hi,

pushed the change to: https://github.com/matevy/tuleap.git

PR is fine, probably easier to iterate at first

Yes, makes sense. Sorry for complicating.

Please say which variant is better for you, gerrit or PR on github. If gerrit is OK, I can set it up, will need it anyway later.

This is why I suggested to use a PR on github instead of full blown gerrit right from the beginning.

The thing is that reviewing a .patch in a file is not really effective for me/us.

Hi,

I could, but I think it does not make sense yet. What is currently implemented is only enforcing of permission, no management of configuration over web ui.
Idea why I am asking to have a quick look is just to get confirmation this is the right way to enforce permissions. Sorry, I am not familiar with Tuleap architecture yet. I have seen some deprecated code and I am not sure whether the way I have implemented is in line what future permission model will be.

Could you please submit it as a review on gerrit ?

If you don't want to bother setting up gerrit right now you can push a PR on github as well but be careful, we won't integrate a PR nor we will integrate a whole branch, it needs to be done step by step (commit per commit) through gerrit.

Thanks !

is there any description how permission model works on Tuleap?

Not really, the code is the source of truth. For "display/update" related stuff, it's mainly in plugins/tracker/include/Tracker/FormElement/Tracker_FormElement_Field_PermissionsOnArtifact.class.php file and for enforcement of permissions it's in plugins/tracker/include/Tracker/Permission/PermissionChecker.class.php

Hi Manuel,

is there any description how permission model works on Tuleap? I am still looking for per Artifact setting of permission (that is setting permission for individual artifact). If I get access to that scenario, then it will be easy to apply just the same to individual comment/attachment of the artifact.

thanks,
Matevz

>That should be the way permissions are managed elsewhere in Tuleap: select which user groups should have access to comment (as an option, pretty >much as it's done for Artifact permissions.

So you suggest each comment should have a small icon which would open a larger selection window/box to select which individual group within the tracker should have visibility of the comment?
That might be clumsy but clean architectural implementation.
Is there an example how you do it for other items for example each individual artefact? It seems on my setup I am unable to set permissions per each individual artefact. Just for the whole tracker.

What about granularity of visibility?
Would just boolean do? That would give us Visible to all and Visible just for internal users?

Or does it make more sense to establish some kind of visibility level?
Or even more complicated where it would be possible to select which user groups view the comment and which not? Or even per user... but that is really way of.

That should be the way permissions are managed elsewhere in Tuleap: select which user groups should have access to comment (as an option, pretty much as it's done for Artifact permissions.

I think adding configuration to each tracker where one can create 1 or more visibility levels would do.
And then assign to each visibiliy level per tracker which groups can view this visibiliy level.

No go for me, no configuration at tracker level, esp. this way that is done nowhere in Tuleap yet.

What about granularity of visibility?
Would just boolean do? That would give us Visible to all and Visible just for internal users?
Or does it make more sense to establish some kind of visibility level?
Or even more complicated where it would be possible to select which user groups view the comment and which not? Or even per user... but that is really way of.

I think adding configuration to each tracker where one can create 1 or more visibility levels would do.
And then assign to each visibiliy level per tracker which groups can view this visibiliy level.

Here is a good place as it allows to keep a nice history record of the discussion. For side questions https://chat.tuleap.org is a good alternative (but as it's slack, cannot be used for any kind of percistency)

Hi,

yes I am. Where should we do the talking to not pollute this artifact by perhaps some stupid questions from my side?

Could I code it in such a way it would be acceptable for everybody ?

You're welcome !

We didn't really thought about the implemenation yet but if your are willing to tackle this, we can definitively help / guide.

Hi,

This is very interesting feature also for me. Could I code it in such a way it would be acceptable for everybody? Have you perhaps already discussed how it could/should be implemented?

Regards,
Matevz

Hi, thank you.

One more example is about defect management. When your client submits a defect (during the development phase of a project), you would like to discuss with your dev team about it. Perhaps, you require them to fix the defect in a given way. You could also add a comment to remind you (the dev team) of a decision you took to fix the defect.

In summary, you have the client that submits the defect at one side and at the other side the dev team that want to discuss about the defect and the technical solution without the client.

Hello

One example is HelpDesk,  some  discussion may stay internaly to the support team, not seen by the requester.

Regards

Laurence

Hi, thanks for the suggestion. Could you please detail the business need and give us some real cases where it would be useful?