Skip to content

Conversation

Zhao-Andy
Copy link
Contributor

@Zhao-Andy Zhao-Andy commented Mar 13, 2019

This PR enables the org pro dashboard view. I decided to leave refactoring and making more data available for the next PR.

Copy link
Contributor

@lightalloy lightalloy left a comment

Choose a reason for hiding this comment

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

Thanks for the refactoring 👍 I noticed it's work in progress, but still couldn't resist commenting )

end

def last_week_reactions_count
Reaction.where(reactable_id: user_article_ids, reactable_type: "Article").where("created_at > ? AND created_at < ?", 2.weeks.ago, 1.week.ago).size
Copy link
Contributor

Choose a reason for hiding this comment

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

It's tempting to create private methods like reactions_scope and comments_scope for the repeating parts, just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think I'll do that. There's definitely a better way to organize the mass amount of data.

@Zhao-Andy Zhao-Andy marked this pull request as ready for review March 18, 2019 13:29
@Zhao-Andy Zhao-Andy changed the title WIP Org Pro Dashboard Backend Org Pro Dashboard Backend Mar 18, 2019
@Zhao-Andy Zhao-Andy changed the title Org Pro Dashboard Backend Enable Org Pro Dashboard View Mar 18, 2019
Copy link
Contributor

@maestromac maestromac left a comment

Choose a reason for hiding this comment

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

👍

@maestromac maestromac merged commit 8a3d3ae into forem:master Mar 18, 2019
@pr-triage pr-triage bot added the PR: merged bot applied label for PR's that are merged label Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants