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.

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

P0: Intake 'Objective' Flag for Topcoder Users #3281

Closed
ajefts opened this issue Aug 26, 2019 · 20 comments
Closed

P0: Intake 'Objective' Flag for Topcoder Users #3281

ajefts opened this issue Aug 26, 2019 · 20 comments
Assignees
Milestone

Comments

@ajefts
Copy link
Contributor

ajefts commented Aug 26, 2019

Objective: Add a data flag ("Customer Project Request" vs. "Demo/Test") for Topcoder user roles to indicate their purpose for submitting project intake. This flag will serve two purposes. First, it will aid ensuring that we have cleaner data for internal analytics purposes. Second, it will serve the routing of new projects alerts to BDRs/AEs.

Requirements:

  • If a Topcoder user role (admin/manager/copilot) accesses a project intake form, when the intake form loads for these users they should see the following on the first screen of each intake form:

  • Question Title: "Intake Purpose"

  • Checkbox Answer Options:

    • "Submitting Client Request"

    • "Internal Project"

    • "Demo/Test/Other"

Note: This should be a required question and answer for all Topcoder users who are submitting an intake form.

@ajefts
Copy link
Contributor Author

ajefts commented Aug 26, 2019

@vikasrohit vikasrohit modified the milestones: Connect 2.4.14, 2.4.15 Aug 26, 2019
@ajefts ajefts changed the title Intake 'Objective' Flag for Topcoder Users P0: Intake 'Objective' Flag for Topcoder Users Sep 5, 2019
@maxceem
Copy link
Collaborator

maxceem commented Sep 11, 2019

As I understand, this question should be stored in some filed like:

details.intakePurpose

With the possibles values:

  • client-request
  • internal-project
  • demo-test-other

If project is being created by a customer user and we don't show this question, should we set the value of this field by default to details.intakePurpose = "client-request"?

@maxceem maxceem added the Have A Question Developer asked some question to copilot/manager. label Sep 11, 2019
@vikasrohit
Copy link

Yes, we are thinking of something similar, however, the problem is that we don't yet have support for hide/show of a question based on user role. Can you please create a separate task for that and get it done with the bug bash or may you directly.

@maxceem
Copy link
Collaborator

maxceem commented Sep 12, 2019

I was thinking to hardcode such a question into the create form, though supporting it as a general ability to hide question depend on the user role sounds better to me.

I'll migrate permission method from Project Service for checking permissions and will add the support for such questions myself or via bug bash.

@vikasrohit
Copy link

Cool. I didn't get where and why we need to migrate the permission method?

@maxceem
Copy link
Collaborator

maxceem commented Sep 13, 2019

I didn't get where and why we need to migrate the permission method?

In Project Service we created reusable method to check permissions described in the issue General permission methods

We also, have a similar method client-side but it's different so I was thinking to introduce new functionality which depends on the user permissions using new general method from server-side (copy it to client side and adapt if necessary).

@vikasrohit
Copy link

Okay. I think we should do it with logic we have right now without much refactoring. we would do refactoring later.

@maxceem maxceem self-assigned this Sep 13, 2019
@maxceem maxceem removed the Have A Question Developer asked some question to copilot/manager. label Sep 13, 2019
@maxceem
Copy link
Collaborator

maxceem commented Sep 13, 2019

I propose to add a new property userPermissionCondition which would make a node (section, subSection, question or option) to be shown only if user match the permission rule defined in userPermissionCondition like this:

userPermissionCondition = { "topcoderRoles": ['administrator', 'Connect Administrator', 'Connect Copilot', 'Connect Manager', 'Connect Account Manager', 'Connect Copilot Manager'] }

Questions:

  1. We already have condition property which also hides question on the condition. Should we somehow be able to combine these two conditions? There are 3 ways:
    1. don't support both conditions together for one "node"
    2. if at least one condition match - we show the "node" (OR logic)
    3. if at least one condition fails - we hide the "node" (AND logic)
  2. For the purpose of this task, I guess it's best to use a section, so first we only ask one Intake Purpose question, and after answering and clicking Next it should got hidden, and we go to the next screen with main form questions.
  3. Should we show the answer to this questions on the summary pages?

@vikasrohit
Copy link

@maxceem I am still processing the userPermissionCondition question in my mind as I am having slight confusion about exposing the user roles in the templates from a security perspective. For others, here are my answers:

  1. Yes
  2. I think we can leave it on our current logic, which I think would show it automatically? If I am wrong here, please correct me what all we need to do to render this question on summary pages. As I don't have answers for them yet, I am just willing to leave them as is (if it shows the intake question or not).

@vikasrohit
Copy link

@maxceem separate issue #3300 for the same question rendering for customer roles.

@maxceem
Copy link
Collaborator

maxceem commented Sep 16, 2019

  1. I think we can leave it on our current logic, which I think would show it automatically?

On the summary page we explicitly list the indexes of sections to show. So if we keep this question on a separate section, we can decide if we want to show it or no on the summary section. The concern I have regarding showing it is that we have a title and intro text which is shown on the 2nd section, so the intake question would be shown on top of them like this https://monosnap.com/file/0BdVrDhV16ovoTUccMDv1MT8yh3xBg.

@vikasrohit
Copy link

Okay. Got it. I think we are good then, we can control visibility of this new section via template itself, right? So, we can hide it if the position of the section on the summary screen looks odd to the product managers, and we can show it anytime they want it. So, I guess, no changes needed for this.

@maxceem
Copy link
Collaborator

maxceem commented Sep 18, 2019

I think we are good then, we can control visibility of this new section via template itself, right?

Right.

@maxceem
Copy link
Collaborator

maxceem commented Sep 18, 2019

I am still processing the userPermissionCondition question in my mind as I am having slight confusion about exposing the user roles in the templates from a security perspective.

@vikasrohit do you have any other ideas how would like to handle it? I like to use this approach because of its universality as we've started using this approach for defining permissions everywhere.

If you strictly don't want to expose the name of permissions in templates (even though we expose them in the source code). We can create some simpler custom values for this property. We can name the property userType and support values like:

  • customer
  • topcoder

And we would define internally in the source code, which set of permissions each of these values mean.

@vikasrohit
Copy link

If you strictly don't want to expose the name of permissions in templates (even though we expose them in the source code)

Exposing permissions name is not bad but exposing the exact role name might be risky (though I can not think of exact risk). And you are right, we are exposing the role names in our public source code.

Lets go ahead with your current suggested approach of using roles directly in the new permission field.

@maxceem maxceem added the P0 label Sep 18, 2019
maxceem added a commit that referenced this issue Sep 18, 2019
…OnEdit"

Now when portal subSection type shows section it take into account if original section is not hidden by condition or hiddenOnEdit

This is to support issue #3281
maxceem added a commit that referenced this issue Sep 18, 2019
@maxceem
Copy link
Collaborator

maxceem commented Sep 18, 2019

@vikasrohit it's in done and deployed to DEV via two commits:

For testing you can use the next link which has intake question: https://connect.topcoder-dev.com/new-project/app_new_intake
You can use the next users to see the difference:

  • pshah_manager - see question
  • pshah_customer - doesn't see question

Note, that as for customer the question is hidden, so such question doesn't have any value and thus is not defined at all in project details data.

Let me know if it works good for you, and I'll take care about the other issue #3300

@vikasrohit
Copy link

@maxceem does the new condition usePermissionCondition works only for section right now? if that can work on a question, we can add two questions in the first section, one of which is visible for customers(for #3300) and another for topcoder emplyees (for this issue).

@maxceem
Copy link
Collaborator

maxceem commented Sep 19, 2019

It should work for all kind of nodes, same like condition. I'll give it a try with the other issue #3300

@vikasrohit
Copy link

Great!! I guess then it would work. Let me know if you see any issue with it.

@vikasrohit
Copy link

Working as per requirements on production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants