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

Fix/double requests #8959

Merged
merged 3 commits into from
Jun 1, 2021
Merged

Fix/double requests #8959

merged 3 commits into from
Jun 1, 2021

Conversation

octavioamu
Copy link
Contributor

@octavioamu octavioamu commented May 27, 2021

Description

remove query on view load of grant explorer, instead just use the js request for the api.

Refers/Fixes

Fixes: #8846

Testing

Copy link
Contributor

@gdixon gdixon left a comment

Choose a reason for hiding this comment

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

Looking awesome @octavioamu!! 🚀

@@ -294,6 +294,7 @@ def increment_view_count(self, pks, content_type, user_id, view_type, retry: boo
:param view_type:
:return:
"""
print(pks, content_type, user_id, view_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: dropping this print

@@ -595,6 +595,11 @@ def get_grants(request):
del grant_json['weighted_risk_score']
grants_array.append(grant_json)

pks = list([grant.pk for grant in grants])
if len(pks):
print(request.user)
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one

@@ -867,6 +872,7 @@ def grants_landing(request):

def grants_by_grant_type(request, grant_type):
"""Handle grants explorer."""
print(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one

@thelostone-mc
Copy link
Member

Could we remove the print and move this to master as opposed to stable ?
codewise LGTM

@octavioamu
Copy link
Contributor Author

Allll done

@thelostone-mc thelostone-mc changed the base branch from stable to master June 1, 2021 05:54
@thelostone-mc thelostone-mc merged commit ae59996 into master Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove gitcoin grants double data fetch requests
3 participants