Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pivot aggregate issue #566

Closed
7 tasks done
ruslandoga opened this issue Feb 11, 2022 · 30 comments
Closed
7 tasks done

pivot aggregate issue #566

ruslandoga opened this issue Feb 11, 2022 · 30 comments
Assignees

Comments

@ruslandoga
Copy link
Contributor

ruslandoga commented Feb 11, 2022

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

@akhrail1996
Copy link
Collaborator

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:

  1. Done
  2. Left to be done
  3. Left to be done (I suggest moving users contacts manually to private page, where possible; and adding fallback email contacts otherwise)
  4. Done
  5. Done
  6. Done
  7. Done

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

@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
Copy link
Collaborator

  1. 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.
    Alternatively we can just "force upgrade" older versions (which will require updating the min_version_requirement on backend once new version becomes available).

  2. 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
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

@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
Copy link
Collaborator

akhrail1996 commented Feb 15, 2022

agree 👍

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

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

  1. 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.

@ruslandoga
Copy link
Contributor Author

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
Copy link
Collaborator

  1. 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
Copy link
Collaborator

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
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

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
Copy link
Collaborator

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
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

@akhrail1996

defp add_urls_to_labels(labels) do
Enum.map(labels, fn label ->
if answer = label["answer"] do
if url = Media.known_sticker_url(answer) do
Map.put(label, "url", url)
end
end || label
end)
end

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
Copy link
Collaborator

akhrail1996 commented Feb 15, 2022

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

by icons I mean these:
image

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

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

@akhrail1996
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

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

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

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 15, 2022

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

@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
Copy link
Contributor Author

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

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 17, 2022

@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
Copy link
Collaborator

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
Copy link
Collaborator

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

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 18, 2022

@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
Copy link
Contributor Author

ruslandoga commented Feb 18, 2022

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Contributor Author

@akhrail1996 this seems to be done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants