Addition of Contribute Section in Home of Slack NestBot #593
Conversation
There was a problem hiding this comment.
Good work but it seems you've overdone it a bit.
We already have Issue entity in the index which is essentially a building block for Contribute page.
The tasks' aim here was to reuse slack/commands/contribute.py code by refactoring it into a shared handler similar to chapters.
BTW here @nitinawari suggested the right approach to solve this.
Please remove redundant functionality and owasp app API changes while keeping essential changes only.
|
Ok i'll do it |
There was a problem hiding this comment.
hey @AbhayTopno,
Great work! I can see the effort you've put in, but it seems like there was a slight misunderstanding of the original issue.
When you have time, please check out these suggestions:
I understand what you did — you created a new entity in apps/owaspas Contributor. However, we don’t need to create a new one. We already have issue entities in apps/github. Please use those instead.
You can refer to this comment for the actual steps to resolve this.
Hints for you:
-
You don’t need to make any changes outside of
apps/slack. -
This is not related to this PR, but please make sure to create a separate branch for your PRs in the future, rather than pushing directly to the main branch. don't close this PR until get merged
I hope these comments and suggestions help you resolve the issue.
Thank you!
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| from apps.owasp.models.chapter import Chapter | |||
| from apps.owasp.models.committee import Committee | |||
| from apps.owasp.models.contribute import Contribute | |||
There was a problem hiding this comment.
we don't need to do this because we have alreadyissue model added in apps/github , revert changes for this file
There was a problem hiding this comment.
we already have issue entity in apps/github/api , use that don't create this one
| @@ -4,12 +4,14 @@ | |||
|
|
|||
| from apps.owasp.api.chapter import ChapterViewSet | |||
| from apps.owasp.api.committee import CommitteeViewSet | |||
| from apps.owasp.api.contribute import ContributeViewSet | |||
There was a problem hiding this comment.
we already have github/index/issue use that don't create duplicate
There was a problem hiding this comment.
don't create this contribution model we already have apps.github/model/issue.py utilize that
There was a problem hiding this comment.
remove this not part of task we already have issue in github/models/managers`
There was a problem hiding this comment.
same goes for this also
| ): | ||
| """Get contribute blocks.""" | ||
| from apps.owasp.api.search.issue import get_issues | ||
| from apps.owasp.models.contribute import Contribute |
There was a problem hiding this comment.
plz use issue model from apps/github/models don't create extra model , it is a duplicate
| from apps.owasp.models.contribute import Contribute | |
| from apps.github.models.issue import Issue |
There was a problem hiding this comment.
please write required test cases for new files and update for modified files
|
Thanks for the correction. I'll make the changes as you suggested, and sorry for the trouble I'm causing @nitinawari |
|
Hey @AbhayTopno, no need to apologize for anything. We’re all in the learning phase. 👍 If you need any help, feel free to reach out to me on Slack DM. I’ll be happy to assist. |
|
@nitinawari thanks for helping with reviews! |
Finished adding the Contribute section to the home of Slack NestBot. If there are any conflicts, feel free to reach out to me