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

Add PlacePage donations #88

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Add PlacePage donations #88

merged 1 commit into from
Nov 26, 2024

Conversation

rtsisyk
Copy link
Member

@rtsisyk rtsisyk commented Oct 22, 2024

https://docs.google.com/document/d/1f2zjYBr24Wlh1vBcTtCmisx-HidaYwivkXZGCHjlTTY/edit

curl -H "X-OM-DataVersion: 241001" -H "X-OM-AppVersion: 2024.10.22-10-Google" -H 'Accept-Language: de-DE' https://meta-dev.omaps.workers.dev/maps | python3 -m json.tool

{
    "servers": [
        "https://cdn.organicmaps.app/",
        "https://cdn-uk1.organicmaps.app/",
        "https://cdn-nl1.organicmaps.app/",
        "https://cdn-de2.organicmaps.app/",
        "https://cdn-de3.organicmaps.app/"
    ],
    "settings": {
        "DonateUrl": "https://organicmaps.app/donate/",
        "NY": "false"
    },
    "productsConfig": {
        "placePagePrompt": "Organic Maps ist dank deiner Spenden f\u00fcr alle kostenlos. Keine Werbung. Keine Tracker. Open Source.",
        "products": [
            {
                "title": "4,99\u20ac pro Monat",
                "link": "https://donate.stripe.com/6oEg2912f2c81r200j"
            },
            {
                "title": "34,99\u20ac pro Jahr",
                "link": "https://donate.stripe.com/eVa2bjdP19EA0mY6oI"
            },
            {
                "title": "Andere",
                "link": "https://donate.stripe.com/fZe9DLdP14kgedO28t"
            }
        ]
    }
}

@rtsisyk rtsisyk requested a review from kirylkaveryn October 22, 2024 10:11
src/locales.json Outdated
@@ -0,0 +1,7 @@
{
"fr": {
"placePagePrompt": "Faites un don pour soutenir le développement d'Organic Maps",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Эти строки обязательно слать с метасервера, или можно разово вбить в strings.txt, как было сделано с донатами?

}
const parts = locale.split(/[-_]/);
const language = parts[0].toLowerCase();
const country = parts[1] ? parts[1].toUpperCase() : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему страну нельзя брать из заголовков CF, как сейчас берётся континент?

@@ -14,6 +14,10 @@ npm i

Use `npx wrangler dev` for localhost development and for testing using Cloudflare dev tools.

```
curl -H "X-OM-DataVersion: 241001" -H "X-OM-AppVersion: 2024.10.22-10-Google" -H 'Accept-Language: fr-FR' http://localhost:8787/maps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Какая вероятность возмущённых комментов по поводу сбора дополнительной инфы о юзерах при добавлении отправки ещё одного хидера на сервер в открытых PR в основной репо? Можно ли без этого обойтись, чтобы не привлекать ненужное внимание к уже отправляющимся хидерам?

Copy link
Collaborator

@kirylkaveryn kirylkaveryn Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@biodranik если не слать локаль, то придется каждому юзеру качать всю эту инфу в полном объеме для всех языков (все переводы и все продукты) и фильтровать ее уже на этапе парсинга данных на устройстве. Если там будет много стран, то json будет приличный... да и не зачем на девайс приходить тому что не нужно...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Значение страны юзера уже есть в заголовках хидеров CloudFlare. Можно просто взять и использовать if (cf.country == 'France'). Чем явная передача с клиента лучше?
  2. Чем передача строчек переводов с сервера лучше явного вбивания их на клиенте через strings.txt?

Copy link
Collaborator

@kirylkaveryn kirylkaveryn Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. если есть возможность не слать с клиента было бы лучше конечно!
  2. строки промоушен текста+продукты и их ссылки такая инфа которую бы хорошо менять динамически отвязавшись от апдейта приложения и не ожидая весь цикл разработка-ревью-установка юзером (может занять месяц/ы). Пока у нас карты не обновляются так, но я думаю что в будущем мы и это сделаем! Можно settings чекать при каждом запуске и всегда иметь up to date содержание баннера, постепенно добавлять страны и конфигуровать текста гибче. - например к какому-нибудь празднику как с елочкой у нас).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Не слать легко, но придётся сразу выбрать тексты и переводы для всех языков, или при необходимости выкатить обновление.
  2. Не очень понял про продукты. Разве тут не достаточно получения списка цен и его отображения (а потом передачи значений в виде параметров при клике)? Самое простое было бы "что пришло в жсоне, то и показать, и то отправить назад". Например, прилетело $ 20 для кнопки, оно и отображается, а на сервер улетает что-то вроде https://url/buttonclicked?value=$%2020.
  3. Чекать при каждом запуске пока не стоит, ОМ не ходит лишний раз в интернет, и это хорошо, т.к. не привлекает внимания provacy-focused community.
  4. Нам не нужно спамить юзеров сразу после установки ОМ, наоборот, лучше показывать только тогда, когда человек действительно пользуется некоторое время. Поэтому не вижу проблем из-за задержек (если изначальные тексты составлены достаточно хорошо).

@rtsisyk
Copy link
Member Author

rtsisyk commented Oct 29, 2024

Значение страны юзера уже есть в заголовках хидеров CloudFlare. Можно просто взять и использовать if (cf.country == 'France').

Спасибо за интересную идею, которая ранее не приходила в голову. Хорошо узнать, что Cloudflare умеет возвращать страну регистрации IP адреса, с которого ведется обращение к серверам. Уверен, что они используют максимально актуальные GeoIP данные. Единственная сложность в использовании данного подхода заключается в том, что пользователи иногда путешествуют по миру, зачастую скачивая всякие различные приложения с картами (например, MAPS.ME) находясь уже в поездке. Если условный француз путешествует в условный Тайлайнд, то ему всё еще следует показывать тексты донатов на французском, валюту EUR и суммы уровня Франции, но точно никак не тексты на тайском с суммами в батах.

Какая вероятность возмущённых комментов по поводу сбора дополнительной инфы о юзерах при добавлении отправки ещё одного хидера на сервер в открытых PR в основной репо?

Выражение эмоций через возмущённые комментарии является неотъемлемой частью человеческого сознания. Избежать подобного рода обратной связи невозможно даже если совсем ничего не делать. Трудно сказать, что потенциально может вызывать большее возмущение — появление новой кнопки для сбора донатов на весьма заметном месте, отправка какого-то там стандартный Accept-Language HTTP header для показа пользователю контента на его языке, или же сам факт обнаружения проприетарного backend для remote config на устройстве пользователя. Стоит заметить также, что при всём при этом никакие данные как не собирались, так и не собираются. Наверное, стоит открыть код метасервера, чтобы исключить спекуляции об обратном. Параллельно стоит заменить закрытую vendor-lock in технологию от Big Evil Tech Corp на понятным всем nginx или его альтернативу, ибо про это тоже могут быть возмущённые комментарии.

В общем и целом обсуждаемые здесь проблемы крайне интересны, но за деревьями не видно леса. Важное по данной фиче заключается в том, что тестируются новые кнопки в новом месте (Place Page). UI, конечно, очень примитивный, но уже лучше, чем ничего. Даже текущая реализация уже позволяет добавлять страны и менять суммы без прохождения review, хотя первоначальный план зашить JSON для 3-5 стран прямо в приложение. Всё это еще не позволяет сделать подписки, т.к. отсутствует идентификация пользователя и обратная связь о подписках из платёжной системы. С большой долей вероятности после тестирования кнопок в place page вся самодельная машинерия для отправки сумм и текстов будет отправлена на свалку и заменена на Apple In-App Purchases.

@biodranik
Copy link
Member

Замечание про юзера в путешествии валидное.

Но если не слать сразу все переводы (или хотя бы в добавок дефолтный английский), или не хранить сразу все переводы на устройстве, то при переключении языка/локали устройства будет отображаться текст на старом языке, пока юзер не попробует что-то скачать с метасервера и снова не запросит строчки.

@rtsisyk
Copy link
Member Author

rtsisyk commented Nov 5, 2024

Но если не слать сразу все переводы (или хотя бы в добавок дефолтный английский), или не хранить сразу все переводы на устройстве, то при переключении языка/локали устройства будет отображаться текст на старом языке, пока юзер не попробует что-то скачать с метасервера и снова не запросит строчки.

Осталось понять почему это важно.

@rtsisyk rtsisyk force-pushed the placepage_donations branch 4 times, most recently from b582c36 to 189d3dc Compare November 16, 2024 14:55
@rtsisyk
Copy link
Member Author

rtsisyk commented Nov 16, 2024

Обновил ссылки.

@rtsisyk
Copy link
Member Author

rtsisyk commented Nov 16, 2024

Управление подписками на https://account.organicmaps.app/. Пытаюсь настроить stripe, чтобы он добавлял данную ссылку в инвойсы и письма. Пока можем по письмам в поддержку отписывать.

@rtsisyk
Copy link
Member Author

rtsisyk commented Nov 16, 2024

image

image

image

@biodranik
Copy link
Member

Страницы донатов и подписок, которые увидят пользователи, перейдя по ссылке, будут отображаться на их языке?

Не очень хорошо, что в текущем публичном PR добавлен хидер с языком пользователей. Вариант с сервера слать сразу все имеющиеся переводы (с фоллбэком на english) был отброшен? Почему?

@kirylkaveryn
Copy link
Collaborator

kirylkaveryn commented Nov 20, 2024

@vng @biodranik @rtsisyk давайте решим что делать с данными, а то это блочит мой ПР с парсингом донейшенов:

  1. шлем все данные (переводы + донейшены) и фльтруем на клиенте по локали
  2. добавляем accept lang header в реквест и билдим prodictsConfig json на сервере

@biodranik
Copy link
Member

Я за вариант 1: его проще реализовать и поддерживать как на клиенте так и на сервере, он более приватный, и не палит публично остальные отправляемые на метасервер хидеры.

@vng
Copy link
Member

vng commented Nov 21, 2024

Если честно, я не вижу проблем ни в 1 ни в 2.
В 1 например мы можем спалить дотошному юзеру: А почему вы на рашку показываете 5 евро, а на Швейцарию 25 :)

@kirylkaveryn
Copy link
Collaborator

kirylkaveryn commented Nov 21, 2024

Тогда попробую резюмировать плюсы и минусы для обоих решений:

1. шлем все данные (переводы + донейшены) и фльтруем на клиенте по локали

Pros:

  • 1 минимум работы на сервере
  • 2 простая реализация
  • 3 не палим сервер
  • 4 не шлем на сервер доп данные с клиента (более приватный)

Cons:

  • 1 на клинет летят данные не нужные ему (даже если в его локали не поддерживаюся донаты на РР) которые тоже нужно парсить (чтобы убедиться что страны нету в списке)
  • 2 увеличивает обьем передаваемых данных (возможно придется архивировать json productsConfig). Чем сложнее будут настройки экрана/ов донейшена, тем больше проблем может возникнуть потому ка кобьем данных будет расти оч сильно даже вводя 1 новую строку
  • 3 если слать переводы промптов и увеличивать кол-во стран с донатами, то п2 еще более актуален
  • 4 если не слать переводы промптов, а зашить их в клиент (слать только список донейшенов), то для обновления промптов нужно проходить весь релизный цикл (около месяца сейчас)
  • 5 продолжение п4 - нет возможности обновлять промпты к какому-либо евенту (как мы делаем с елочкой на НГ)
  • 6 если хранить переводы промптов на девайсе - нужно сразу делать их все (даже если страна не поддерживает строки) чтобы не было коллизий при вкл/выкл страны в конфиге

2. добавляем accept lang header в реквест и билдим prodictsConfig json на сервере

Pros:

  • 1 минимум работы на клиенте
  • 2 простая реализация
  • 3 на клиент приходит только то, что запрошено, никаких лишних данных
  • 4 если нету поля в конифге - ничего не будет парсится
  • 5 маленький обьем передаваемых данных - не нужна логика с архивированием и легко расширяется при изменении ремоут конфига для донейшен экрана
  • 6 не зависим от релизного цикал если надо что-то поправить/обновить к конкреной дате/евенту

Cons:

  • 1 может спалить сервер привлекая внимание к доп хидеру!
  • 2 больше работы на сервере по вычитке локали и сборке json с конфигом
  • 3 на сервере наверное чуть сложнее поддерживать всю систему чем на клиенте
  • 4

@vng
Copy link
Member

vng commented Nov 21, 2024

Вне зависимости от варианта, тексты промта и все переводы идут с сервера, а не зашиваются в аппу. Иначе смысл тогда всего этого ..

Если сейчас уже готов вариант 2 на серваке (насколько я понял он готов, да?) - останавливаемся на нем.
Не вижу опасения, что такого там "палит" Accept-Language.

@vng
Copy link
Member

vng commented Nov 26, 2024

@rtsisyk Добавить US/UK?

@rtsisyk rtsisyk force-pushed the placepage_donations branch from 189d3dc to b89edcd Compare November 26, 2024 12:45
@rtsisyk rtsisyk force-pushed the placepage_donations branch from b89edcd to ea6f4d6 Compare November 26, 2024 13:31
@rtsisyk rtsisyk merged commit ea6f4d6 into master Nov 26, 2024
1 check passed
@rtsisyk rtsisyk deleted the placepage_donations branch November 26, 2024 13:32
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

Successfully merging this pull request may close these issues.

4 participants