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

Added Conflicting Claims count pill on Claims listing #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prathambatra
Copy link
Contributor

Signed-off-by: prathambatra [email protected]
fixes #399
Screenshot from 2020-06-06 01-40-09
Screenshot from 2020-06-06 01-40-19
Screenshot from 2020-06-06 01-40-25

For optimizing, I have reduced db queries by using the fact if a claim is a conflict to another claim then vice versa is also true. conflictKey is used for the same type of claims and for all these claims only 1 db count query is performed. if there are no conflicts, no badge is shown.

@boss-contributions-bot
Copy link

Thanks @prathambatra, for opening the pull request! 🙌

One of our mentors will review the pull request soon. ✅

Star ⭐ this project and tweet 🐦 about your contributions.

@prathambatra
Copy link
Contributor Author

@hereisnaman

issueUrlDetail = getUrlDetails(claim["issueUrl"])
conflictKey = issueUrlDetail.project + '/' + pullUrlDetail.type + '/' +issueUrlDetail.id
if(conflictCounts[conflictKey] === undefined) {
const c = await du.getConflictsCount(claim,issueUrlDetail,pullUrlDetail.type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can a query inside a loop be a single query. Try to write a separate raw query for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so you mean, there is no need for a loop just a query wich groups all conflicts for all claims. ??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@prathambatra
Copy link
Contributor Author

prathambatra commented Jun 13, 2020

@hereisnaman I just realized that there is no need for a DB query because we are querying for all the claims on claims/view page. so we can use our conflict logic to find conflicts count without any query. We can do this in linear time (total number of claims) using a hashmap. should I proceed in this way?

@thenamankumar
Copy link
Contributor

@hereisnaman I just realized that there is no need for a DB query because we are querying for all the claims on claims/view page. so we can use our conflict logic to find conflicts count without any query. We can do this in linear time (total number of claims) using a hashmap. should I proceed in this way?

The claim view page is paginated, why are we querying all the claims?

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 count pill on Claims listing
2 participants