Page MenuHomePhabricator

Remove Legacy impression counting code
Open, Needs TriagePublic

Description

If the new implementation works reliably (the data in both tables looks the same), we should remove the existing code that picks up banner impression data from the IMAP inbox and change all SQL queries to use the new database tables.

We can also remove the custom Docker image (with the IMAP extension) from the docker-compose file, delete the custom Dockerfile and use our officially built image. Then we can also use make ci instead of composer ci in Travis without performance hits and with the benefit of being able to use the kontocheck library in CI (current tests omit it).

Implementation details

This is the "mapping" of impression changes for the SQL (old table on the left, new one on the right):

backend_impressionsbanner_impressions
banner_idbanner_tracking_id
datetimeinterval_start
imp_countimpression_count
backend_bannerbanner_tracking
banner_idid
cn_namebanner_name
layoutN/A (deprecated)
campaigncampaign
keywordkeyword

Remove

  • application/Commands/FetchImpressionsCommand.php
  • src/DataAccess/DoctrineEntity/BackendImpression.php - this is now BannerImpression
  • src/DataAccess/DoctrineEntity/BackendBanner.php - this is now BannerTracking
  • Banner::getBannerMapping() and its private method from application/model/Banner.php
  • Impression::storeImpression and Impression::extractTimestamp from application/model/Impressions.php

Adapt SQL queries:

  • Impression::getImpressionsPerHour and Impression::extractTimestamp from application/model/Impressions.php
  • application/model/Analysis.php search for table and column names and replace them
  • WMDE\Fundraising\Backend\BannerImpressionRepository::getBannerImpressions - remove and use WMDE\Fundraising\Backend\BannerImpressions\ImpressionRepository instead
  • ToolsHelpers::getImpressionCount - ask Kai if the script using the ToolsHelper method - campaignExport.php is still needed and what purpose it serves
  • Discuss deletion of DatabaseInstallationTest - dubious value of testing the db setup for the test environment.
  • test/Data/files/database/banners.sql needed for the analysis test, replace with a dump from the new tables

Related Objects