Skip to content

Refactoring LiveMetrics to avoid photo.album_id requirement.#3356

Merged
ildyria merged 1 commit intomasterfrom
refactoring-livestats
Jun 8, 2025
Merged

Refactoring LiveMetrics to avoid photo.album_id requirement.#3356
ildyria merged 1 commit intomasterfrom
refactoring-livestats

Conversation

@ildyria
Copy link
Copy Markdown
Member

@ildyria ildyria commented May 17, 2025

This pull request introduces significant changes to the metrics and event handling system, alongside updates to request handling for improved flexibility and consistency. Key changes include the creation of base classes for metrics events, the addition of a from_id attribute for better tracking, and refactoring of query logic in metrics retrieval.

Event System Refactoring:

  • Introduced BaseAlbumMetricsEvent and BasePhotoMetricsEvent classes to centralize shared logic for album and photo-related metrics events, such as the key() method and toArray() method. [1] [2]
  • Updated existing event classes (AlbumDownload, AlbumShared, AlbumVisit, PhotoDownload, PhotoFavourite, PhotoShared, PhotoVisit) to extend the new base classes, removing redundant key() method implementations. [1] [2] [3] [4] [5] [6] [7]

Request Handling Enhancements:

  • Added a new HasFromId interface and HasFromIdTrait to standardize the handling of a from_id attribute in requests. [1] [2]
  • Updated ZipRequest and PhotoMetricsRequest to implement HasFromId, enabling the use of from_id for additional context in metrics-related operations. [1] [2]

Metrics Query Optimization:

  • Improved query logic in GetMetrics.php by adding a safeguard to exclude null album_id values and simplifying conditions for visit actions.
  • Removed unused commented-out code in the select clause for cleaner and more maintainable queries.

Controller Updates:

  • Updated controllers to include from_id in event dispatch calls, ensuring that this additional context is passed to metrics events. [1] [2] [3] [4]

Miscellaneous:

  • Simplified the logic in LiveMetricsResource by removing fallback logic for album_id and photo URLs, assuming data consistency.

@ildyria ildyria requested a review from a team as a code owner May 17, 2025 20:25
@ildyria ildyria added the Review: medium Medium review expected: not many files, some attention to details required. label May 17, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.03%. Comparing base (e813c38) to head (9e97597).

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ildyria ildyria force-pushed the refactoring-livestats branch from e5811d5 to a745953 Compare May 18, 2025 09:30
@ildyria ildyria force-pushed the refactoring-parent-id branch from 3703cc3 to e813c38 Compare May 26, 2025 19:27
@ildyria ildyria force-pushed the refactoring-livestats branch from a745953 to 9e97597 Compare May 26, 2025 19:28
@ildyria ildyria force-pushed the refactoring-parent-id branch from e813c38 to 273a3f3 Compare May 28, 2025 16:44
@ildyria ildyria force-pushed the refactoring-livestats branch from 9e97597 to 659305c Compare May 28, 2025 16:44
Base automatically changed from refactoring-parent-id to master May 28, 2025 18:18
@ildyria ildyria force-pushed the refactoring-livestats branch from 659305c to 877f658 Compare May 28, 2025 18:21
@ildyria ildyria force-pushed the refactoring-livestats branch from a921deb to ecc738c Compare May 28, 2025 18:32
@ildyria ildyria merged commit 5484dee into master Jun 8, 2025
34 checks passed
@ildyria ildyria deleted the refactoring-livestats branch June 8, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review: medium Medium review expected: not many files, some attention to details required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants