pivot aggregate issue - Githubissues

ruslandoga commented 2 years ago

mirroring https://github.com/getsince/test5/issues/208:

[x] private profile pages: store, save (incl. dealing with blurred photos created locally on frontend), manage (return blurred to feed & real to matches) https://github.com/getsince/test3/pull/571
[x] upgrade pushes
[x] #602
[x] seen (profile, pages) data collection for future algorithmic https://github.com/getsince/test3/pull/567
[x] probably record likes to the above as well https://github.com/getsince/test3/pull/567
[x] track opening of clickable stickers https://github.com/getsince/test3/pull/573
[x] backend-generated stories: news + voting platform https://github.com/getsince/test3/pull/575

akhrail1996 commented 2 years ago

mirroring getsince/test5#208:

[ ] private profile pages: store, save (incl. dealing with blurred photos created locally on frontend), manage (return blurred to feed & real to matches) add private stories #571

[ ] upgrade pushes

[ ] maybe move existing contact labels & even ever shared contacts (?) to private pages (?)

[ ] seen (profile, pages) data collection for future algorithmic add seen timings #567

[ ] probably record likes to the above as well add seen timings #567

[ ] track opening of clickable stickers track sticker click #573

[ ] backend-generated stories: news + voting platform add news #575

From what I understand:

Done
Left to be done
Left to be done (I suggest moving users contacts manually to private page, where possible; and adding fallback email contacts otherwise)
Done
Done
Done
Done

ruslandoga commented 2 years ago

@akhrail1996 what needs to be done for 2 and 3? how pushes need to be upgraded? how contact stickers look like in json? I can turn text labels with contact info into contact stickers if I know the new contact sticker structure.

akhrail1996 commented 2 years ago

2) Push-notification for users trying to do no-longer supported actions (from #560) saying that these actions are deprecated and asking to upgrade the app. Rationale: even if we keep supporting them on backend, users with the newest app version won't receive them. Alternatively we can just "force upgrade" older versions (which will require updating the min_version_requirement on backend once new version becomes available).

3) Current dicts (Kind.Type is String enum).


var dict: [String: Any] = [
            "position": [position.x, position.y],
            "zoom": zoom,
            "rotation": rotation,
            "icon": icon.absoluteString
            "text": text
            "kind": kind.rawValue
        ]

switch kind {
case .phone, .email: ()

case .deeplink:
    dict["deeplink"] = deeplink!.absoluteString
    dict["default_url"] = defaultURL!.absoluteString
}

References: StoryLabelContact: https://github.com/getsince/test5/blob/pivot-to-tiktok-for-profiles/Since/Story/Models/StoryLabelContact.swift StoryLabel.toDict(): https://github.com/getsince/test5/blob/pivot-to-tiktok-for-profiles/Since/Story/Models/StoryLabel.swift#L177 How Contact sticker is opened (phone & email is not done yet): https://github.com/getsince/test5/blob/pivot-to-tiktok-for-profiles/Since/Feeds/FeedCoordinator.swift#L352 Deeplinks & default urls: https://github.com/getsince/test5/blob/pivot-to-tiktok-for-profiles/Since/StoryEdit/Views/Sticker%20Selection/Inputs/ContactInputVC.swift#L169

So an example would be:

{"icon": "https://d20pncrvwjzpw9.cloudfront.net/Telegram?d=3304cb2e968756dbace49c4d6f81c3d8",
"kind": "deeplink",
"text": "@gugugui", 
"zoom": 1.4656478783421008,
"deeplink": "tg://resolve?domain=gugugui",
"position": [70.3593604062259, 213.23329034850553],
"rotation": -21.53808336101298,
"default_url": "https://t.me/gugugui"}

ruslandoga commented 2 years ago

@akhrail1996 "icon" and "text" should be removed (there's already "value" in other labels), why are there both "default_url" and "deeplink"? "kind" is too broad and doesn't seem to be necessary. Why not reuse sticker structures (question, answer, etc.)?

{
  "question": "telegram",
  "answer": "https://t.me/gugugui",
  "value": "@gugugui",
  "zoom": 1.47,
  "position": [70.36, 213.23],
  "rotation": -21.54
}

https://t.me/apple-app-site-association telegram supports universal links

akhrail1996 commented 2 years ago

agree 👍

icon, default_url and deeplink were designed to support new contact-types in future, but I think this is not so important

ruslandoga commented 2 years ago

Push-notification for users trying to do no-longer supported actions (from https://github.com/getsince/test3/pull/560) saying that these actions are deprecated and asking to upgrade the app. Rationale: even if we keep supporting them on backend, users with the newest app version won't receive them.

We can release a new minor version of the app with support for backend-supplied error messages, if we do it now, I think many users would be on that version when the new major version comes out, and we'd be able to reply with descriptive error messages when they perform deprecated actions. I say no to both push notifications and force upgrade.

ruslandoga commented 2 years ago

icon, default_url and deeplink were designed to support new contact-types in future, but I think this is not so important

what was the logic behind that design?

akhrail1996 commented 2 years ago

Push-notification for users trying to do no-longer supported actions (from remove interactions #560) saying that these actions are deprecated and asking to upgrade the app. Rationale: even if we keep supporting them on backend, users with the newest app version won't receive them.

We can release a new minor version of the app with support for backend-supplied error messages, if we do it now, I think many users would be on that version when the new major version comes out, and we'd be able to reply with descriptive error messages when they perform deprecated actions. I say no to both push notifications and force upgrade.

Good idea. Let's do it. I can release this version today.

akhrail1996 commented 2 years ago

icon, default_url and deeplink were designed to support new contact-types in future, but I think this is not so important

what was the logic behind that design?

logic was that everything is fetched from backend => doesn't depend on locally saved icons for contacts, locally created deeplink paths for different contact-types, etc.

ruslandoga commented 2 years ago

icons urls were always coming from backend but never stored there, they are rendered in the view layer. nothing media related should be stored in the app; as for kind = deeplink, did you want to be able to add new contact types without upgrading the app? so that even thought the current version doesn't know about tiktok deeplink, having the kind=deeplink and default_url=https://tiktok.com/asdfasdf set it would still be able to open it? this can be made possible with sticker-structure

akhrail1996 commented 2 years ago

icons urls were always coming from backend but never stored there

could you please send the link to code? cannot seem to find them

ruslandoga commented 2 years ago

@akhrail1996 https://github.com/getsince/test3/blob/44c3752abc0e4f1e2800c51c6b420151130d1320/lib/t_web/views/view_helpers.ex#L49-L57

anything that's stored in the story is replaced by whatever Media.known_sticker_url(answer) returns, which are these stickers: https://backend2.getsince.app/admin/stickers

akhrail1996 commented 2 years ago

@ruslandoga there are no icons ("question") there, only stickers ("answer")

by icons I mean these:

ruslandoga commented 2 years ago

@akhrail1996 https://d20pncrvwjzpw9.cloudfront.net/Telegram?d=3304cb2e968756dbace49c4d6f81c3d8 this is a sticker, it's in icon, so it's an icon?

akhrail1996 commented 2 years ago

did you want to be able to add new contact types without upgrading the app? so that even thought the current version doesn't know about tiktok deeplink, having the kind=deeplink and default_url=https://tiktok.com/asdfasdf set it would still be able to open it?

yes

this can be made possible with sticker-structure

great, how would you design it?

akhrail1996 commented 2 years ago

@akhrail1996 https://d20pncrvwjzpw9.cloudfront.net/Telegram?d=3304cb2e968756dbace49c4d6f81c3d8 this is a sticker

yes, this is a sticker that we no longer use. (but not an icon)

ruslandoga commented 2 years ago

same way that the web works, https:// or http:// schema = link -> deeplink

http://example.com <- automatically is recognised as a link

ruslandoga commented 2 years ago

yes, this is a sticker that we no longer use. (but not an icon)

so why are you bringing them up? or why is it called icon in your payload?

akhrail1996 commented 2 years ago

yes, this is a sticker that we no longer use. (but not an icon)

so why are you bringing them up?

I never brought up stickers, only icons. Icons are currently stored only locally and not returned from backend. Adding a new contact-type requires a new icon. If icons are stored locally then there is no way to add a new contact-type without upgrading the app. That was the logic behind original StoryLabelContact design (answering your question).

However this (adding new contact-type support) is not of great importance now. I think we can release this version without it. In future we will return all sticker categories (together with corresponding icons & answer options) from backend.

ruslandoga commented 2 years ago

it is important right now since if you release a version with that structure it'd need to be supported and I'd need to change backend, which can be easily avoided with a bit of thought

akhrail1996 commented 2 years ago

@akhrail1996 "icon" and "text" should be removed (there's already "value" in other labels), why are there both "default_url" and "deeplink"? "kind" is too broad and doesn't seem to be necessary. Why not reuse sticker structures (question, answer, etc.)?
{
  "question": "telegram",
  "answer": "https://t.me/gugugui",
  "value": "@gugugui",
  "zoom": 1.47,
  "position": [70.36, 213.23],
  "rotation": -21.54
}
https://t.me/apple-app-site-association telegram supports universal links

I agreed to this structure earlier (so plan to release with it), or do you want to make some changes to it?

ruslandoga commented 2 years ago

no, it's perfect since it doesn't add anything new

ruslandoga commented 2 years ago

@akhrail1996 checked_profiles table now contains profiles with has_text_contact? :: bool column.

select count(*) from checked_profiles where "has_text_contact?"; -- 401
select count(*) from checked_profiles; -- 3020
select count(*) from profiles; -- 3020
select count(distinct from_user_id) from match_interactions where data ->> 'type' = 'contact_offer'; -- 379

select count(*) from (
  select user_id from checked_profiles where "has_text_contact?"
  union
  select distinct from_user_id from match_interactions where data ->> 'type' = 'contact_offer'
) as c; -- 644

akhrail1996 commented 2 years ago

644 / 3020 = 21% is pretty good

I looked at the users active in Feb:

select count(*) from profiles where last_active > '2022-02-01 00:00:00'; -- 1103

SELECT
    count(*)
FROM (
    SELECT
        *
    FROM (
        SELECT
            user_id
        FROM
            checked_profiles
        WHERE
            "has_text_contact?"
        UNION
        SELECT DISTINCT
            from_user_id
        FROM
            match_interactions
        WHERE
            data ->> 'type' = 'contact_offer') AS c) p
    INNER JOIN LATERAL (
        SELECT
            user_id
        FROM
            profiles
        WHERE
            last_active > '2022-02-01 00:00:00'
            AND user_id = p.user_id) pip ON TRUE; -- 503

so 503 / 1103 = 45% is even better

akhrail1996 commented 2 years ago

@ruslandoga do you need help in converting them to contact stickers? (the manual part)

ruslandoga commented 2 years ago

@akhrail1996 no need, I'll write another tool to help with conversion. But I'll need your help on deciding what to do with text labels containing more than one contact. There are multiple "Пишите мне на тг/инст @durov" labels, and there's one user with three contacts in a single text label.

ruslandoga commented 2 years ago

select count(*) from profiles where story is not null; -- 1987
select count(*) from profiles where story is not null and last_active > '2022-02-01 00:00:00'; -- 895

So it's more like 30% and 55%.

akhrail1996 commented 2 years ago

@akhrail1996 no need, I'll write another tool to help with conversion. But I'll need your help on deciding what to do with text labels containing more than one contact. There are multiple "Пишите мне на тг/инст @durov" labels, and there's one user with three contacts in a single text label.

I think we should keep the info fully, so 2 contacts -> 2 stickers, 3 contacts -> 3 stickers probably with a small zoom value, one below another

akhrail1996 commented 2 years ago

select count(*) from profiles where story is not null; -- 1987
select count(*) from profiles where story is not null and last_active > '2022-02-01 00:00:00'; -- 895

So it's more like 30% and 55%.

even better, meaning that we are indeed making something users already want

the numbers of null stories are very high, that's why new approach to onboarding is important as well imo

ruslandoga commented 2 years ago

@akhrail1996 this seems to be done

getsince / test3

pivot aggregate issue #566