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

Conflict Analysis on Claim Review #392

Conversation

YashKumarVerma
Copy link
Contributor

Fixes #260
Follow up of #279 as tree became too messed up.

  • Shows tab wise
    • List of claims which submitted the same issue
    • List of claims which submitted the same PR
    • List of claims which submitted both items same.
    • Three separate tabs with numbering to inform about linked claims
    • Tabs don't show when no linked/conflicting claims present.

Underlying Idea

With #390 sent, now we can create a more robust conflict view.
Reasons:

  • There are multiple PRs sent for the same issue. we need to list them.
  • Shows number of PRs for single issue submitted on BOSS portal
  • image
  • There are
    • 100 claims which have the same URL submitted for both handles.
    • 140 claims which have different URLs submitted

Only displaying claims which submit the same issue and same PR may not provide full context.
Admins must have a complete view of where all were the issues and pull requests submitted


UI

screencapture-localhost-3232-claims-20716-2020-05-31-01_47_06

Zoom
image

@YashKumarVerma YashKumarVerma changed the title Conflicting Analysis on Claim Review Conflict Analysis on Claim Review May 30, 2020
@YashKumarVerma
Copy link
Contributor Author

@hereisnaman

@thenamankumar
Copy link
Contributor

To merge this PR the one I asked for earlier has to be merged first. This PR doesn't take care about different url formatting. Secondly you are pulling all the data upfront when only some part of it is required.

@YashKumarVerma
Copy link
Contributor Author

To merge this PR the one I asked for earlier has to be merged first. This PR doesn't take care about different url formatting.

Thanks for the review.
The reason that it does not accommodate the irregularities (ending hash, spaces, slashes) is that once the pr which fixes the URLs is done, no new occurrences of any such irregularity would arise, and the older ones would be fixed too.

Secondly you are pulling all the data upfront when only some part of it is required.
Can you please elaborate this one? is it about getting all details of the claims on the view ?

@thenamankumar
Copy link
Contributor

To merge this PR the one I asked for earlier has to be merged first. This PR doesn't take care about different url formatting.

Thanks for the review.
The reason that it does not accommodate the irregularities (ending hash, spaces, slashes) is that once the pr which fixes the URLs is done, no new occurrences of any such irregularity would arise, and the older ones would be fixed too.

Secondly you are pulling all the data upfront when only some part of it is required.
Can you please elaborate this one? is it about getting all details of the claims on the view ?

On a re look it seems fine to have the current clause.

About loading data. In getConflictsReport I can see you are loading data for issues, pull and both. When you are only consuming partial data from that.

@YashKumarVerma
Copy link
Contributor Author

YashKumarVerma commented May 31, 2020

To merge this PR the one I asked for earlier has to be merged first. This PR doesn't take care about different url formatting.

Thanks for the review.
The reason that it does not accommodate the irregularities (ending hash, spaces, slashes) is that once the pr which fixes the URLs is done, no new occurrences of any such irregularity would arise, and the older ones would be fixed too.

Secondly you are pulling all the data upfront when only some part of it is required.
Can you please elaborate this one? is it about getting all details of the claims on the view ?

On a re look it seems fine to have the current clause.

About loading data. In getConflictsReport I can see you are loading data for issues, pull and both. When you are only consuming partial data from that.

Consuming partial data as in ? The view renders the array that the queries return. If they return an empty array, the views don't display anything.

Are you referring to making 3 queries (a separate query to get both) ?
I'm sorry I'm unable to grasp 😢

@prathambatra
Copy link
Contributor

Fixes #260
Follow up of #279 as tree became too messed up.

  • Shows tab wise

    • List of claims which submitted the same issue
    • List of claims which submitted the same PR
    • List of claims which submitted both items same.
    • Three separate tabs with numbering to inform about linked claims
    • Tabs don't show when no linked/conflicting claims present.

Case 2 and 3 are not required at all. mainly conflicted claims are those where there is same issue and multiple PRs (both pull or new issue case) are there. I haven't seen any rejected claim where a person has used someone else's pull request URL and that too on some not-related issue. That's too impractical to happen in real scenario.

@YashKumarVerma
Copy link
Contributor Author

I think Case 2/3 will be triggered when someone submits the two URLs interchanged, i.e. pull request in issues column, and issueUrl in the pull request column. That would trigger case 1 as well, no doubts to that. It's just a method to specifically point it out.

With the inputs and history URLs clean, I think we can remove it.

@prathambatra
Copy link
Contributor

I think Case 2/3 will be triggered when someone submits the two URLs interchanged, i.e. pull request in issues column, and issueUrl in the pull request column. That would trigger case 1 as well, no doubts to that. It's just a method to specifically point it out.

With the inputs and history URLs clean, I think we can remove it.

Yes, This check should be at add page. We shouldn't allow incorrect claims in any way.

@YashKumarVerma
Copy link
Contributor Author

I think Case 2/3 will be triggered when someone submits the two URLs interchanged, i.e. pull request in issues column, and issueUrl in the pull request column. That would trigger case 1 as well, no doubts to that. It's just a method to specifically point it out.
With the inputs and history URLs clean, I think we can remove it.

Yes, This check should be at add page. We shouldn't allow incorrect claims in any way.

thanks for pointing out 😄

@thenamankumar
Copy link
Contributor

To merge this PR the one I asked for earlier has to be merged first. This PR doesn't take care about different url formatting.

Thanks for the review.
The reason that it does not accommodate the irregularities (ending hash, spaces, slashes) is that once the pr which fixes the URLs is done, no new occurrences of any such irregularity would arise, and the older ones would be fixed too.

Secondly you are pulling all the data upfront when only some part of it is required.
Can you please elaborate this one? is it about getting all details of the claims on the view ?

On a re look it seems fine to have the current clause.
About loading data. In getConflictsReport I can see you are loading data for issues, pull and both. When you are only consuming partial data from that.

Consuming partial data as in ? The view renders the array that the queries return. If they return an empty array, the views don't display anything.

Are you referring to making 3 queries (a separate query to get both) ?
I'm sorry I'm unable to grasp 😢

You are making 3 queries and after fetching checking which one to use.

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

Successfully merging this pull request may close these issues.

Add conflicting claims section on accept claims page
3 participants