-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
Feature/convert2grant #7359
Feature/convert2grant #7359
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7359 +/- ##
==========================================
- Coverage 26.41% 26.40% -0.01%
==========================================
Files 306 306
Lines 30136 30157 +21
Branches 4455 4460 +5
==========================================
+ Hits 7959 7964 +5
- Misses 21902 21923 +21
+ Partials 275 270 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://user-images.githubusercontent.com/19514207/87412921-2b78f800-c5fc-11ea-9509-91e40793edde.jpg
^ The screenshot don't match the design on the ticket
Padding / text align / button color and so on!
Could we keep the code as close to the design as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sanchaymittal lets keep the existing winner banner where it is and move grants image to the left side. What does green Edit button do?
But then it will overlap with the edit button if I move the grant tag to the left side. The green edit button is for the project owner to edit the project. |
Just for context -convo w @sanchaymittal -- we'll keep it like this for now until @zoek1 builds project V2 when we will clean the card up as part of that push. @thelostone-mc @octavioamu |
@sanchaymittal could you post a video or an ngrok tunnel so that we can play with the feature? |
yes, would like a video or live demo - that would be perfect! |
Video demo: https://www.loom.com/share/abe0eb4176e34b599442b2fcdd733b11 cc: @PixelantDesign |
Additioanlly calling out that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a bunch of comments
how is a project denoted as a grant in our DB? is there a flag that tell us if it was converted to a grant, and when it occurs? |
A field is created in HackathonProject DB grant_link which is a foreign Key to Grant. Once a grant is created through convert2grant flow then it updates the field with grant object. |
got it, so when the grant is created via convert2grant flow, that means the if yes, then LGTM, we can track the outcome of this feature |
Yes, when we create the grant we save the project object to the foreign key field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments !
8e17842
to
67a1e39
Compare
PR is failing because migrations |
- To share the project params with intra view functions
ordering of projects - winners, grants, created_on, id
- winner ribbon conflicting with grant tag. - fetch project_pk
- After making change to grant_link - API call to DB doesn't fetch the updates - if queried with filter then updated data is fetched only
- revert the changes done in last commit
- change the styling close to the design - rmv the unwanted logic
- padding change for ribbon tag to make it smaller - change the position of grant tag to left
eb3a787
to
e384bc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment on indentation, otherwise LGTM
@sanchaymittal this is something I would your help as a follow up PR against zcash branch https://gitcoincore.slack.com/archives/G01CNNGMT3K/p1602241297000100
Noted! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments. the rest seems looking good
rnm: rename data to project_data
Description
Convert to Grant feature
Video: https://www.loom.com/share/abe0eb4176e34b599442b2fcdd733b11
Refers/Fixes
#7098
Testing