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_impressions | banner_impressions |
---|---|
banner_id | banner_tracking_id |
datetime | interval_start |
imp_count | impression_count |
backend_banner | banner_tracking |
---|---|
banner_id | id |
cn_name | banner_name |
layout | N/A (deprecated) |
campaign | campaign |
keyword | keyword |
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