Content view type in RDLC and configuration flag for keep alive - #62
Conversation
New public ContentViewType enum (text, video, tts, audio) which can be passed when reporting content page view. The value is carried inside the client object of the RDLC parameter. The field is optional, so RDLC stays byte-identical for hosts which do not report a view type. Reporting a page view which is not article content clears the previously reported value, mirroring how contentPageViewSource is handled.
New shouldTrackContentKeepAlive flag, enabled by default, so existing hosts behave as before. When disabled, reporting content page view no longer starts the keep alive manager, which means no measurement timers and no keepAlive events. The contentKeepAliveDataSource parameter is now optional. Disabling keep alive also disables automatic effective page view reporting, which is driven by keep alive measurements.
Dropping the default value from contentKeepAliveDataSource means a new call site has to decide about keep alive instead of silently ending up without it. Existing callers are unaffected, they pass a non optional value. Skipping KeepAliveManager.start also has to stop the previous content's measurement, because start is what flushes and resets it. Without that, keep alive kept reporting for content the user had already left — reachable now that the data source can be nil while tracking is enabled.
e995abb made ContentMarkAsPaid build source.id from contentSpaceUuid, but the expected values in the tests kept using contentId, so ten tests have been failing on master since then. Expected values are recomputed from each test's own metadata. The keep alive test named after paid content was reporting unpaid content, which made it a copy of the unpaid one, so it now uses paidContent: true.
| @@ -15,6 +15,10 @@ struct Client: Encodable { | |||
| struct ClientType: Encodable { | |||
There was a problem hiding this comment.
to już nie jest client type tylko ClientContext albo coś takiego - ja bym rename zrobił
There was a problem hiding this comment.
Zrobione ClientType -> ClientContext
|
|
||
| import Foundation | ||
|
|
||
| final class ClientDecorator: Decorator { |
There was a problem hiding this comment.
po przejściu na budowanie wartości w fabryce dekorator znowu wnosi wyłącznie client.type, więc zostawiam ClientDecorator
|
|
||
| eventsService?.updateUniqueIdentifier(partiallyReloaded: partiallyReloaded) | ||
| eventsService?.updateStructureType(structureType: .structurePath(currentStructurePath), contentPageViewSource: nil) | ||
| eventsService?.updateViewType(nil) |
There was a problem hiding this comment.
hmm ja bym teraz do tego podszedl inaczej - bo sie to zmienia z globalnego dekoratora na coś co jest decydowane per zdarzenie, a nie per stan w aplikacji
bo to tylko (ja tak to rozumiem):
czyli client.type, na PV dodatkow możesz rozszerzyć o dodatkowy context czyli
ma być tylko pod page view i content page view
a dla kazdego innego zdarzenia tego nie ma
Więc:
- ja bym zrezygnował z tego dekoratora całkowicie
- dodawał to pole w factory w każdym zdarzeniu po prostu i tam wszedzie dał bez tego (tylko client type) a dla content page view w zależności od przekazanego typu widoku
There was a problem hiding this comment.
Zrobione zgodnie z tym, co ustaliliśmy. Dekorator został, bo zdarzenia customowe hosta (reportEvent z gotowym Event) nie przechodzą przez fabrykę i straciłyby RDLC - teraz wnosi sam client.type.
Rozszerzoną wartość buduje fabryka w createPageViewEvent, a parametr ustawiony przez zdarzenie nie jest już nadpisywany przez dekorator. Sprawdziłem, że dziś nie ma ani jednej kolizji kluczy między dekoratorami a fabrykami, więc żadne istniejące zdarzenie nie zmienia wartości.
| partiallyReloaded: Bool, | ||
| contentKeepAliveDataSource: RingPublishingTrackingKeepAliveDataSource) { | ||
| contentKeepAliveDataSource: RingPublishingTrackingKeepAliveDataSource?, | ||
| viewType: ContentViewType? = nil) { |
There was a problem hiding this comment.
to by pasowało dać przed pageVieeSource
There was a problem hiding this comment.
Przestawione: viewType jest drugi, zaraz po contentMetadata
| // Closing the previous content's measurement is normally done by `KeepAliveManager.start`, so skipping | ||
| // it here has to stop that measurement explicitly — otherwise it keeps reporting for content the user | ||
| // has already left. | ||
| guard configuration?.shouldTrackContentKeepAlive == true else { |
There was a problem hiding this comment.
zrobienie samego dataSource opcjonalnego jest odpowiednikiem tego - nie dodawałbym jawnie falgi już bo to nic nie wnosi nam - jest data source to raportujemy, nie ma to nie
There was a problem hiding this comment.
Flaga wyleciała. Został nullowalny contentKeepAliveDataSource - bez wartości domyślnej, żeby nowe wywołanie musiało świadomie zdecydować.
Dodałem przy tym stop() na ścieżce z nil: start() sam woła stop() na nowej treści, więc bez tego pomiar poprzedniego artykułu leciałby dalej.
…ator The view type describes the page view it was reported with and is not inherited by the events which follow it, so it no longer lives as decorator state. ClientDecorator goes back to carrying the client alone, which keeps RDLC on every event, including the generic ones a host builds itself and which never reach a factory. The page view event builds its own extended RDLC, and a parameter set by an event is no longer overwritten by a decorator. No factory sets a key any decorator also sets, so nothing else changes value. ClientType is renamed to ClientContext, since it no longer describes the type alone, and the keep alive configuration flag is gone — an optional data source already says whether there is anything to measure.
Values agreed with analytics: today an article has three presentation modes and there is no audio or video one, while a smart short is a mode of its own rather than a separate content kind.
…-type-and-keep-alive-flag Conflict resolution: - ClientContext.swift keeps the renamed model and takes over the 'variant' field and the 'ClientVariant' type from the deleted ClientType.swift, so 'Client' now carries both the view type and variant.external - ClientDecorator keeps the variant.external handling from the base branch and builds the client model through its new initializer - Changelogs/1.13.0 lists the entries from both branches - Example project references the source and test files added on the base branch, which the test target compiles explicitly
After the merge two places produced 'RDLC': 'ClientDecorator' for every event and 'EventsFactory' for a page view carrying a view type. Since an event keeps its own value, a content page view reported with a view type dropped the variant.external parameters the host had set. 'ClientDecorator' now owns the client model alone and builds the per-event value, so the page view event carries both.
2 participants
Two changes, both backwards compatible.
Merged with the base branch after #63, in two commits — conflict resolution first, then the one behaviour change it needed. Both branches produced RDLC, so a content page view reported with a view type would have dropped the variant.external parameters; ClientDecorator now builds the per-event value and that event carries both. Changelogs/1.13.0 lists the entries from both branches.
The Example project now also references the source and test files added by #63. Its test target compiles the SDK sources from an explicit list, so without them the target does not build at all.
Tests: 130 executed, 0 failures.
Also fixed the 10 RDLCN assertions which had been failing on master since e995abb changed source.id to contentSpaceUuid — expected values are recomputed from each test's own metadata, and the keep alive test named after paid content now actually reports paid content instead of being a copy of the unpaid one.
Version bumped to 1.13.0 in the podspec.