samvera-deprecated / sufia

[DEPRECATED] Sufia: a fully featured, flexible Samvera repository front-end.
http://sufia.io/
Other
111 stars 78 forks source link

Revisit analytics implementation to account for new FileSet/GenericWork split #1455

Closed jcoyne closed 8 years ago

jcoyne commented 8 years ago

now that we've moved to works/file_sets, what analytics should be collected? On what events?

awead commented 8 years ago

Related tickets #1419 #1388

mjgiarlo commented 8 years ago

Here's where I stand: we should collect pageviews for the work (on hits to the work show page) and we should collect downloads for on the fileset (on clicks that result in downloads, whether from the work show page, fileset show page, or catalog pages if downloading is accessible there). We should continue to cache analytics in the application database. We should present all analytics information in the /works/{id}/stats view. Let me know if I missed anything.

gordonleacock commented 8 years ago

Okay, I'm going to start poking at this one. However, I can not currently assign myself to it. Mike - can you permit me so I can do that. Thanks!

mjgiarlo commented 8 years ago

@gordonleacock I've added you to the GitHub team now. You'll need to check your email for the invitation.

gordonleacock commented 8 years ago

Jose and I worked on this today and discovered that the _show_actions.html.erb partial was trying to use if @presenter.display_feature_link? and if @presenter.display_unfeature_link?. Those were causing sufia to die with a method not found error. For now, we have commented them out. Do folks agree that they don't belong in this partial code in sufia 7?

jcoyne commented 8 years ago

@gordonleacock I think @hectorcorrea just added that code, so I suspect it does belong in Sufia 7.

gordonleacock commented 8 years ago

@mjgiarlo I did a fresh pull this afternoon and it was dying (no method found) on line 16: <% if @presenter.display_feature_link? %>

We'll refresh our code tomorrow and try it again. Thanks.

hectorcorrea commented 8 years ago

@gordonleacock this is the code where the display_feature_link? and display_unfeature_link? were introduced a few days ago: https://github.com/projecthydra/sufia/pull/1503/files

gordonleacock commented 8 years ago

We didn't touch "app/presenters/sufia/work_show_presenter.rb". We changed the partial in "app/views/curations_concerns/base/_show_actions.html.erb".

gordonleacock commented 8 years ago

I've pulled down the code and rechecked the problem. It still exists. Unlike sufia/app/presenters/work_show_presenter.rb, the file sufia/app/presenters/file_set_presenter.rb does not have the methods _display_featurelink? and _display_unfeaturelink?.

@hectorcorrea, @mjgiarlo, @jcoyne: Should it have those methods? It seems like it should because the app/views/curations_concerns/base/_show_actions.html.erb file is used by both the generic_work page and the file_set page, so if I remove them, then the generic work page will be missing the featured item functionality.

So I think Jose and I should add those methods to file_set_presenter.rb and have them always return false. Unless it is planned that file sets can be featured. Let me know what you think.

By the way, I wasn't sure if I should add this here or created a new issue. Let me know if that would be better.

hectorcorrea commented 8 years ago

@gordonleacock why don't you push your branch to GitHub (or create a PR) and we can take a look at see what might be different.

gordonleacock commented 8 years ago

Hi Hector, It is now up in: https://github.com/gordonleacock/sufia/blob/file_set_presenter_fix/app/presenters/sufia/file_set_presenter.rb if you approve, I'll issue a PR.

gordonleacock commented 8 years ago

Okay, making good progress on getting works analytics to show. I've got the works analytics page displaying, now I need to get the tracking for works going and showing up on that page. Hope to be able to issue a PR today or early next week.

carolyncole commented 8 years ago

We are suggesting that the cache should be updated to include the information needed to be displayed for a work and that the nightly update could roll up the stats from google analytics for each work.

mjgiarlo commented 8 years ago

@cam156 Do you see the roll-up capability within works as necessary plumbing to get this working in 7.0.0 or a value-add? I was thinking it'd be the latter, which is why I was leaning towards post-7.0.0 for the roll-up feature, but I might be missing something.

mjgiarlo commented 8 years ago

Hi, @gordonleacock, was your analytics PR sufficient to close this issue or is there work remaining on this?

gordonleacock commented 8 years ago

Hi @mjgiarlo - It can be closed. Can/Should I do that? Thanks again for your help!

On Tue, Mar 15, 2016 at 12:54 PM, Michael J. Giarlo < notifications@github.com> wrote:

Hi, @gordonleacock https://github.com/gordonleacock, was your analytics PR sufficient to close this issue or is there work remaining on this?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/projecthydra/sufia/issues/1455#issuecomment-196920553

mjgiarlo commented 8 years ago

@gordonleacock I've got it, np! In the future, you can also leverage the magic of GitHub by including e.g. "fixes #1455" in your commit message. Thank you, sir.

Fixed by #1564