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

Move render_activity_sentence into Activity model #1287

Closed
westonganger opened this issue May 14, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@westonganger
Copy link
Contributor

commented May 14, 2019

I was trying to create a status page for my sites and wanted to display some activity similar to the dashboard site activity log.

The current Activity model is rather sparse in terms of available methods making it difficult to extract usable info from. I dug into it and found out that most of the logic for extracting activity info is within the DashboardHelper#render_activity_sentence making it difficult for me to integrate with my solution.

Could we not move the render_activity_sentence method into the model somehow?

@did

This comment has been minimized.

Copy link
Member

commented May 14, 2019

I was trying to create a status page for my sites and wanted to display some activity similar to the dashboard site activity log.

What kind of extra events are you going to track? Or do you just want a page in the CMS to display all the activities for your sites? Do you have a mockup?

Now, speaking of the implementation. I'd rather keep the model super light.
I also took a look the current implementation. Since we need to call Rails url helpers + DOM helpers, it makes totally sense to use a Rails helper.

Now, if you tell me that we need those methods for another view (within a different controller), I guess we've to move the code related to the display of the activities into its own helper, like Locomotive::ActivityHelper.

How does it sound to you?

@westonganger

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

I originally thought it could be extracted to the model, however after trying a solution I realize the method is fairly tied to the view.

Yes it would be helpful and more intuitive to extract it to its own helper I would think. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.