Skip to content

T239598 : Allow users to customise data granularity for edits over time graph#55

Merged
Jain-Aditya merged 16 commits intoWikipediaLibrary:masterfrom
sanyam-git:master
Feb 4, 2020
Merged

T239598 : Allow users to customise data granularity for edits over time graph#55
Jain-Aditya merged 16 commits intoWikipediaLibrary:masterfrom
sanyam-git:master

Conversation

@sanyam-git
Copy link
Copy Markdown
Contributor

@sanyam-git sanyam-git commented Jan 29, 2020

with reference to T239598
work don :

  • at first the user will be displayed the default view (best suitable).
  • then the user can change the display as per his requirements.
  • the AJAX request for another display is sent when user clicks the button

dailyView
monthlyView
yearly View

@sanyam-git
Copy link
Copy Markdown
Contributor Author

with reference to T207846 : Encode 4-byte unicode properly.
added 'charset' : 'utf8mb4' in database settings in settings/base.py
Screenshot from 2020-01-30 11-43-48

@Samwalton9
Copy link
Copy Markdown
Member

Thanks! Can you split these tasks out into two different pull requests please?

added the commit to another pull request
@sanyam-git
Copy link
Copy Markdown
Contributor Author

@Samwalton9 , moved the task T207846 : Encode 4-byte unicode properly to another pull request. Thanks

Copy link
Copy Markdown
Member

@Samwalton9 Samwalton9 left a comment

Choose a reason for hiding this comment

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

Looking good! I just have one question about some code that I didn't entirely understand :)

@Jain-Aditya
Copy link
Copy Markdown
Collaborator

At this point, it looks like this and the daily and yearly charts doesn't show up.
image
Your changes were fine 2 commits behind where I left my previous comment. You just had to remove the line numbers I mentioned.

@sanyam-git
Copy link
Copy Markdown
Contributor Author

sanyam-git commented Feb 2, 2020

At this point, it looks like this and the daily and yearly charts doesn't show up.
image
Your changes were fine 2 commits behind where I left my previous comment. You just had to remove the line numbers I mentioned.

I was getting the same display, just hard refresh the page to load the latest javascript changes and the display will be fine.

@Jain-Aditya
Copy link
Copy Markdown
Collaborator

Jain-Aditya commented Feb 2, 2020

At this point, it looks like this and the daily and yearly charts doesn't show up.
image
Your changes were fine 2 commits behind where I left my previous comment. You just had to remove the line numbers I mentioned.

I was getting the same display, just hard refresh the page and to load the latest javascript changes.

Yup, working fine!

@Jain-Aditya
Copy link
Copy Markdown
Collaborator

Overall it is looking good! Also could you please modify the CSV download view as per the graphs? Currently it gives edits per day irrespective of the view type.

@sanyam-git
Copy link
Copy Markdown
Contributor Author

@Jain-Aditya I have added the CSV functionality and also added tests for the same.
The checks for my first and second commit looks failed maybe because I committed the remaining changes after them. Next time I will club all the changes in one commit.
But as I see all the changes are checked/passed in the last commit.
I think it is good to go. :)

@Jain-Aditya
Copy link
Copy Markdown
Collaborator

Thanks! I see there is lot of code repeated in the time_csv view and time_statistics_data view. What you could do here is internally call an API in the csv view by passing view_type parameter(You can use requests library) and then create a CSV out of it. This will reduce a lot of lines of code. Could you try that?

@sanyam-git
Copy link
Copy Markdown
Contributor Author

Great idea ! I will try to implement it. Thanks.

@sanyam-git
Copy link
Copy Markdown
Contributor Author

Implemented the code refactor.

writer = csv.writer(response)
writer.writerow([
# Translators: Date on which edit is made.
_('Date'),
Copy link
Copy Markdown
Collaborator

@Jain-Aditya Jain-Aditya Feb 4, 2020

Choose a reason for hiding this comment

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

Just a small change and it will be good to go. CSV header should also be changed to month, date and year accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done 👍

@Jain-Aditya
Copy link
Copy Markdown
Collaborator

Looking good! thanks @sanyam-git for your valuable work on this task.

@Jain-Aditya Jain-Aditya merged commit dfe9383 into WikipediaLibrary:master Feb 4, 2020
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.

3 participants