Feature/life notification #60

danemesis · 2020-04-09T17:53:56Z

No description provided.

…on logic

danemesis · 2020-04-09T17:54:40Z

TODO: Handle mapping of user subscription input from inline keyboard to our subscription strategy

Proladge

Cool stuff is coming!

Proladge · 2020-04-09T20:02:35Z

server/src/bots/telegram/utils/messageRegistry.ts

-    Register(message: string, callback: (bot: any, message: any, chatId: unknown) => any) : MessageRegistry {
-        this._bot.onText(new RegExp(message, 'g'), (message) => callback(this._bot, message, getChatId(message)));
+    public registerMessageHandler(regexp: string, callback: (bot: unknown, message: unknown, chatId: number) => unknown): MessageRegistry {
+        this._messageHandlers[regexp] = callback;


As far as I know. When we use this method for subscribing on "onText". we say that on specific type of message which is set by regex under the line we are going to use concrete callback.

A bit different pattern works for when we subscribe for "callback_query". Here we do it only once. And have to handle choosing the callback by ourselves, since there is no parameter like regex for this kind of subscription.

So everything written above is for making sure we both are understanding node telegram API the same way and correctly.
And my actual proposition is just to remove the line 15. 🇺🇬

I see now. You just moved merged it to be a single method.
But my personal opinion is that If api devs made those methods different. It make sense handling them differently and don't mess everything up.
I mean having those handlers registered explicitly in a different way, we will have much clear vision on what user request we actually handle just from code.
It prolly just my thoughts....

reverted back 🥇

Proladge · 2020-04-09T20:09:30Z

server/src/services/domain/storage.ts

@@ -0,0 +1,31 @@
+import * as firebase from 'firebase';


I love seeing third party services reused within our App.
Firebase ❤️

Proladge · 2020-04-09T20:12:24Z

package.json

@@ -38,6 +38,7 @@
    "country-emoji": "^1.5.0",
    "dotenv": "^8.2.0",
    "express": "^4.17.1",
+    "firebase": "^7.13.2",


danemesis · 2020-04-11T18:19:28Z

partially fixed in #58

Proladge · 2020-04-11T20:28:17Z

@danbilokha What we've talked about in "offline onverstion" is to make sending notifications kinda scheduled process.
I suppose this could be helpful. And pretty simple to reuse
https://www.npmjs.com/package/node-schedule

Proladge · 2020-04-11T20:48:16Z

Going to get back tomorrow.
To do a second round of reviewing.
It's really huge! 🚢

Proladge · 2020-04-12T06:15:32Z

@danbilokha I'm done with reviewing.
🔥 base 💯
Ready to come back once u react on the comments

jmelnych · 2020-04-12T11:18:02Z

server/src/bots/telegram/botResponse/countriesResponse.ts

-        }
-    }
-};
+import {getContinentsInlineKeyboard} from '../utils/keyboard';


what are the code style rules? single quotes or double quotes?

Proladge

Bomba-Rocketa!

Proladge · 2020-04-11T20:05:02Z

server/src/models/constants.ts

@@ -28,7 +28,9 @@ export enum UserMessages {
    CountriesData = 'Countries data 🌍',
    AvailableCountries = 'Countries we track',
    GetAdvicesHowToBehave = 'Advice how not to 😷',
-    MySubscriptions = 'My subscriptions 💌',
+    SubscriptionManager = 'Subscriptions 💌',


I like. The way u add messages. It always goes with a nice smile!
👍

Proladge · 2020-04-11T20:09:50Z

server/src/services/domain/subscribe.ts

@@ -2,7 +2,8 @@ import {TelegramChat} from "../../bots/telegram/models";
 import {getAvailableCountries} from "./covid19";
 import {Country} from "../../models/country.models";
 import {getTelegramUserSubscriptions, setTelegramSubscription} from "../../bots/telegram/services/storage";
-import {SubscriptionType} from "../../models/subscription.models";
+import {SubscriptionType, UserSubscription} from "../../models/subscription.models";
+import {SubscriptionStorage} from "../../models/storage.models";



INGNORE IT
WAS FIXED IN A FUTURE COMMIT

I'd rename the file to be subscription.
Since from my point of view, in this folder and layer as well. we are trying to represent core objects of our application.
So naming it subscription would make it more consistent in terms of understanding the domain

Proladge · 2020-04-11T20:12:30Z

server/src/bots/telegram/services/subscriptionNotifierHandler.ts

+    let userSubscriptionNotifications: Array<UserSubscriptionNotification> = [];
+
+    userSubscriptionsOn
+        .filter(subscription => subscription.active !== false)


I'm really new to fire base. But I'd recommend to move this filter to the firebase request.
I'm also wondering what for do we store inactive subscriptions?

Proladge · 2020-04-11T20:13:56Z

server/src/bots/telegram/services/subscriptionNotifierHandler.ts

+    userSubscriptionsOn
+        .filter(subscription => subscription.active !== false)
+        .forEach((subscription: Subscription) => {
+            if (subscription.type === SubscriptionType.Country) { // TODO: Take into account timezone


This if seems like also could be moved up to the firebase reques

Proladge · 2020-04-11T20:16:35Z

server/src/bots/telegram/services/subscriptionNotifierHandler.ts

+
+                const subscriptionCountryLastInfo: CountrySituationInfo = userSubscriptionCountry[userSubscriptionCountry.length - 1];
+
+                const subscriptionCountryLastUpdate = subscriptionCountryLastInfo


Is there any native way that js could save data to string representation and then parse it back.
I mean without all that noise with dashes and parsing ints?

Proladge · 2020-04-11T20:36:33Z

server/src/bots/telegram/botResponse/unsubscribeResponse.ts

+        getUnsubscribeMessageInlineKeyboard(
+            userSubscription
+                .subscriptionsOn
+                .filter(v => v.active !== false)


Filter goes second time.
Just as a reminder from me to follow DRY 🎗️

Proladge · 2020-04-11T20:38:44Z

server/src/bots/telegram/index.ts

@@ -34,26 +39,35 @@ function runTelegramBot(app: Express, ngRokUrl: string) {
    registry.setBot(bot); // TODO: DO IT COOLER
    registry
        .registerMessageHandler(UserRegExps.Start, startResponse)
+        // Feature: Countries / Country


Yeah it's becoming really huge. Good idea for separating it by features 💯

Proladge · 2020-04-11T20:40:49Z

server/src/bots/telegram/services/keyboard.ts

@@ -32,6 +33,45 @@ export const getAfterCountryResponseInlineKeyboard = (country: string): unknown
    return ik.build();
 };

+export const getSubscriptionMessageInlineKeyboard = (): unknown => {


That's was really cool to see! 💯 🎖️

Proladge · 2020-04-12T06:00:26Z

server/src/bots/telegram/index.ts

+        logger.log('info', message);
+        logger.log('info', args);


Should be better to log message with args as a single object. Just to keep each log record synchronized

Proladge · 2020-04-12T06:10:29Z

server/src/services/domain/subscriptions.ts

+export const getConcreteUserSubscriptions = (
+    chatId: number, allUsersSubscriptions: SubscriptionStorage
+): UserSubscription => {
+    const userSubscriptionKey = Object.keys(allUsersSubscriptions)


Is it possible for us to ask from firebase just a user's subscriptions only and not all of them?

danemesis added 4 commits April 8, 2020 11:16

added firebase.ts

4e62059

POC for reading data from cloud

59d0292

template for subscriptions

9a9c5de

subscription: show inline; set connections with DB; handle subscripti…

cbeb42f

…on logic

danemesis requested a review from Proladge April 9, 2020 17:53

danemesis self-assigned this Apr 9, 2020

danemesis added this to the release 2 milestone Apr 9, 2020

danemesis linked an issue Apr 9, 2020 that may be closed by this pull request

Live notifications #5

Closed

Proladge reviewed Apr 9, 2020

View reviewed changes

danemesis marked this pull request as draft April 10, 2020 04:39

danemesis added 7 commits April 10, 2020 08:51

saving user's subscriptions

c6f6e0d

subscribe (structure, saving it in db); user message; store live updates

ce2a129

sending message to the user? (not sure :D). many, many todo!

56dae37

notifications update rates

d4b8de7

subscribe & send notifications features done

873daf6

unsubscribe functionality

3f10afe

unsubscribe functionality #2

ab32288

danemesis changed the title ~~[WIP] Feature/life notification~~ Feature/life notification Apr 11, 2020

danemesis marked this pull request as ready for review April 11, 2020 18:11

This was linked to issues Apr 11, 2020

When a user clicks on "Countries data" from the Dashboard we have a bug in console #58

Closed

A user UX\UI dashboard navigation #32

Closed

danemesis removed this from the release 2 milestone Apr 11, 2020

unsubscribe functionality #3

7c4ccc7

danemesis added 2 commits April 12, 2020 12:14

fix unsubscribe doesn't work bug

1572334

subscribe \ unsubscribe flow check

b14da89

fix all know issues

e3a356e

danemesis requested a review from jmelnych April 12, 2020 11:12

jmelnych requested changes Apr 12, 2020

View reviewed changes

Proladge reviewed Apr 12, 2020

View reviewed changes

add fetching covid19 updates cron job (from hopkins uni)

e33dfe2

Proladge approved these changes Apr 12, 2020

View reviewed changes

danemesis merged commit 9ee436c into master Apr 12, 2020

danemesis deleted the feature/life-notification branch April 12, 2020 12:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/life notification #60

Feature/life notification #60

danemesis commented Apr 9, 2020

danemesis commented Apr 9, 2020

Proladge left a comment

Proladge Apr 9, 2020

Proladge Apr 9, 2020

danemesis Apr 12, 2020

Proladge Apr 9, 2020

Proladge Apr 9, 2020

danemesis commented Apr 11, 2020

Proladge commented Apr 11, 2020 •

edited

Loading

Proladge commented Apr 11, 2020

Proladge commented Apr 12, 2020

jmelnych Apr 12, 2020

Proladge left a comment

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 11, 2020

Proladge Apr 12, 2020

danemesis Apr 12, 2020

Proladge Apr 12, 2020

danemesis Apr 12, 2020


		const subscriptionCountryLastInfo: CountrySituationInfo = userSubscriptionCountry[userSubscriptionCountry.length - 1];

		const subscriptionCountryLastUpdate = subscriptionCountryLastInfo

Feature/life notification #60

Feature/life notification #60

Conversation

danemesis commented Apr 9, 2020

danemesis commented Apr 9, 2020

Proladge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danemesis commented Apr 11, 2020

Proladge commented Apr 11, 2020 • edited Loading

Proladge commented Apr 11, 2020

Proladge commented Apr 12, 2020

Choose a reason for hiding this comment

Proladge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Proladge commented Apr 11, 2020 •

edited

Loading