Added Conflicted Claims Feature #357
Conversation
Signed-off-by: prathambatra <[email protected]>
|
@hereisnaman please confirm . Do you need the same way conflicted claims below claim card and form on the right side. |
Please refer to the new design and reflect the changes |
|
@hereisnaman cool. I will complete this today. |
…conflicted-claims-2 pulling
Signed-off-by: prathambatra <[email protected]>
Signed-off-by: prathambatra <[email protected]>
|
|
|
@hereisnaman |
routes/root.js
Outdated
| res.send('Error getting conflicting claims') | ||
| }) | ||
| } | ||
| else { |
There was a problem hiding this comment.
There can be conflicts in the case when the pull url points to an issue as well. Handle that here.
There was a problem hiding this comment.
it means same person claimed issue more than once or someone else is claiming issue created by someone. right?
Signed-off-by: prathambatra <[email protected]>
|
|
|
Can you debug why this is not working: |
|
@hereisnaman yeah sure wait. |
|
|
Signed-off-by: prathambatra <[email protected]>
|
@hereisnaman does it show no conflicted claims? |
|
@hereisnaman have you rechecked, as it was working fine for me. |
Signed-off-by: prathambatra <[email protected]>
|
Congratualtions @prathambatra, your pull request is merged! 🎉 Thanks for your contributions and participating in BOSS 2020. 🙌 You can claim your bounty points here. 💰 |
There was a problem hiding this comment.
@hereisnaman @prathambatra I see the testing-required label. I tested this feature
| let urlDetails = { | ||
| project : "", | ||
| type : "", | ||
| id : "" | ||
| } | ||
|
|
||
| url = url.split('/') | ||
| position = url.indexOf('github.com') | ||
|
|
||
| urlDetails.project = url[position+2] | ||
| urlDetails.type = url[position+3] | ||
| urlDetails.id = url[position+4] | ||
|
|
||
| return urlDetails; | ||
| } |
There was a problem hiding this comment.
upperCase lowerCase in the url is not handled properly here.
if github.com is in upperCase then this would fail. (Although you can't really enter https://GITHUB.com/coding-blocks/boss/issues/29923232. this throws duplicate issue error (a bug maybe?). But still it's better to make sure it doesn't break in the future)
Also project, type should be converted to lowerCase.
https://github.com/coding-blocks/BOSS/issues/29923232
https://github.com/coding-blocks/boss/issues/29923232 This fails when these two are the issueUrl. Op.like is case-sensitive I guess
There was a problem hiding this comment.
@prabalsingh24 'GITHUB.com' and 'github.com' should be fixed at add claim level. thanks for pointing the project, and type. yes, they should be converted to lower case.
There was a problem hiding this comment.
@prabalsingh24 @hereisnaman I think we just need to convert both issue URL and pull URL to lowercase before adding. Everything will work fine, no need to do any change anywhere else. just convert url to lowercase on adding.
There was a problem hiding this comment.
Yeah that would work but if there is already a pull url with uppercase, then it might fail in that case. I know chances of that happening is pretty low. But better to be on the safer side and fix it from the both side?
There was a problem hiding this comment.
we will add convert code to add claims and also write a migration to convert existing ones.
|
@prabalsingh24 Can I work on this issue? |
Signed-off-by: prathambatra [email protected]
fixes #260
Created new PR(previous was by master branch) after merging bootstrap changes. UI changes are remaining. will commit soon.