add canAdminister permissions to conversation-thread

[daveconnis]

What's in this PR?

Add the ability for authorized users (admin) to filter by conversation part type.

References #1692

Progress on: #1597

[daveconnis]

Any test that needs to render a conversation is currently failing. At day's end, my thinking is this is happening because I haven't figured out how to write users who are admins into the test because, before this, the tests didn't need users to work. Now that we're working with user permissions, it does.

If you poke around, I haven't defined a user like let user =..., I've mocked a fake "project" and made a user object that includes an admin role.

let project = {
    projectUsers: [
      { role: 'admin' }
    ]
  };

My thought tomorrow was to start by mocking a user, then calling that user inside of project, but now that I write this, it sounds dumb to do that.

[daveconnis]

After reading https://www.npmjs.com/package/ember-can, and then looking again at app/abilities/project.js, I might start tomorrow by putting user=user to the hbs. I don't think I'm giving ability:project what it needs here.

[begedin]

Added some comments.

You don't need to mock out project users, nor do you need to provide a fake user. You already provide a user through the currentUser service anyway.

Look through other tests to see how to mock out an ability directly. That's by far the easiest approach, which doesn't depend on any other mocks.

[begedin]

You removed the rendering of the original message body. This should always be rendered, as far as I know.

[begedin]

Your logic here is flawed. A user who cannot administer will simply be shown all of the conversation parts, regardless of type, except they will be displayed as if they are comments.

You lack a test that would check for this, so you should add that to

test("only comment parts are shown if user cannot administer")

There are multiple ways you could achieve this. The most straightforward would be to rearrange the logic here so it just works, but it would probably make for a complicated and hard to follow template file. Try to think your way through this.

The aim is, if I have a conversation which was commented on twice, then was closed, then reopened

• a user who can administer the project will see a total of 5 parts. 1 for the original message body, 2 comments, 1 closed, 1 reopened
• a user who cannot administer the project will see a total of 3 parts, 1 for the original message body, 2 for comments

Your test should support that goal. The tests you modified are fine. What you're missing are tests for behavior when user is not an admin.

[begedin]

You're using {{#if can 'administer' 'project'}} below, so these two lines are no longer necessary.

[begedin]

Your template does not have access to a project attribute. It has access to a conversation, which is related to a project

{{#if (can "administer project" conversation.project)}}

[daveconnis]

After a long day of me mostly failing/reading guides, tests are still in the same state as yesterday, aka "Any test that needs to render a conversation is currently failing."

With Nikola's help, I've/Nikola has written a filter that returns only comment parts to users who don't have the canAdminister property.

I'm still struggling with placing the new filter in the hbs, I'm just not used to working in it and it feels like I'm just looking at a wall of code. I've been reading guides and going through the codebase to see other examples, but I'm still struggling at the connections. Starting tomorrow, my biggest blocker is simply just placement in the hbs, after that, I have confidence I can knock out the tests pretty easy.

[begedin]

@daveconnis I looked into why your tests are failing. The reason is, the conversation models being created by mirage for the purpose of those tests are created without an associated project.

Look into the mirage factories we have, as well as into how the records in those tests specifically are being created to figure out what the actual problem is.

Remember, if we have a specific project, then any created message needs to belong to that project, and any created conversation needs to belong to that message and that project.

Then, the system figures out if a created user can administer the created project by checking if a projectUser record with a role of "admin" or "owner" exists for that project and that user.

[begedin]

what your saying here is,

• if the message this conversation is related to is not related to any project
  • create a new message and relate it to the existing message as if it were a project
  • also create a project unrelated to anything

This may have somehow fixed the tests, but it definitely doesn't sound correct.

The issue with the failing tests was that conversations were being created without a project, which cannot happen in a real-world scenario.

You were correct in trying to fix the factory, but you probably need to spend some time trying to understand what's going on here.

Your goal is, simply put, that a conversation needs to be created with a project and conversation.project and conversation.message.project need to be equal.

I recommend you experiment with a debugger in the test and the factory itself and see what you get. Check the values of variables on each step of the debugger to see how factories work.

Also look into the documentation for ember-cli-mirage