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

410 file analytics #477

Merged
merged 6 commits into from
May 17, 2023
Merged

410 file analytics #477

merged 6 commits into from
May 17, 2023

Conversation

alishaevn
Copy link
Contributor

Story

We needed to change the method to reference a class that exists, ensuring that the class gets required.

Legato dynamically creates methods based on classes/modules that extend it’s model. And depending on the load sequence of classes, these dynamic methods can vary. By adding an explicit require, we ensure that those methods are properly defined on the profile.

Expected Behavior Before Changes

trying to view the analytics for a file would fail

Expected Behavior After Changes

we can view file analytics

Screenshots / Video

image

resources

alishaevn and others added 3 commits May 16, 2023 14:53
…ring that the class gets required.

Legato dynamically creates methods based on classes/modules that extend it’s model.  And depending on the load sequence of classes, these dynamic methods can vary.
By adding an explicit require, we ensure that those methods are properly defined on the profile.

co-authored-by: jeremy <[email protected]>
@alishaevn alishaevn marked this pull request as ready for review May 16, 2023 20:13
@alishaevn
Copy link
Contributor Author

@orangewolf @jeremyf and I would like your feedback on the solution here. it incorporates the changes from hyrax#5958, but the code still failed without the require statements because the Legato::ProfileMethods.instance_methods weren't finding hyrax__pageviews or hyrax__downloads.

do you have a better solution?

@alishaevn alishaevn requested a review from orangewolf May 16, 2023 20:19
@alishaevn
Copy link
Contributor Author

rob and I paired on the refactor. merging.

@alishaevn alishaevn merged commit 13fc836 into main May 17, 2023
@alishaevn alishaevn deleted the 410-file-analytics branch May 17, 2023 13:14
@alishaevn alishaevn linked an issue Jun 15, 2023 that may be closed by this pull request
1 task
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.

Analytics - File level analytics report
2 participants