Add optional page_type parameter to ParselyMetadata class in iOS SDK
#90
Conversation
MartinAkram
changed the title
page_type parameter to ParselyMetadata class in iOS SDK
MartinAkram
requested review from
HelioMesquita and
cwisecarver
and removed request for
HelioMesquita
|
@cwisecarver I updated the Demo App and built the simulator for iPhone 15 and confirmed |
There was a problem hiding this comment.
This looks good to me. I'd be very tempted, if I were the one doing this, to include all the fields supported by the ParselyUtils metadata class just to get ahead of future requests like this, but if you want to merge this and create a new issue to add the rest of them to both SDKs that's fine too. I just figured getting to grips with the tooling of both SDKs for the time would make adding N fields about the same amount of work as adding one field.
|
@cwisecarver I added the remaining metadata types. Do you mind re-reviewing? I'm pretty sure I got the types right, but a second set of eyes would be great. Thanks so much! |
| @@ -3,42 +3,123 @@ import Foundation | |||
| public class ParselyMetadata { | |||
| var canonical_url: String? | |||
| var pub_date: Date? | |||
| var save_date: Date? | |||
There was a problem hiding this comment.
Sorry, @MartinAkram, my suggestion here was terribly unspecific. Let's drop save_date, custom_metadata, save_date_tmsp, pub_date_tmsp, urls, full_content_word_count, data_source, canonical_hash, canonical_hash64, video_platform, full_content, full_content_sha512, network_id_str, and content_enrichments. Those are all either internal fields that we generate ourselves or they're impractical to send in a pixel like full_content. Sorry for the wasted effort because of my lack of specificity.
There was a problem hiding this comment.
You can get more information about expected types here where the code lives for parsing in-px metadata.
This PR adds a new optional parameter,
page_type, to theParselyMetadataclass in the iOS SDK. This is being done in response to a client question/request (see linked issue below).Companion Android SDK PR: Parsely/parsely-android#102
🤔 Why 🤔
Resolves https://github.com/Parsely/web/issues/13450
✔️ TODOs ✔️