Codecov Report

Patch coverage: 94.53% and project coverage change: +2.83 🎉

Comparison is base (497f3ce) 58.71% compared to head (9f93506) 61.55%.

@@            Coverage Diff             @@
##           master    #1649      +/-   ##
==========================================
+ Coverage   58.71%   61.55%   +2.83%     
==========================================
  Files         128      133       +5     
  Lines        2885     3004     +119     
  Branches      697      759      +62     
==========================================
+ Hits         1694     1849     +155     
+ Misses       1187     1038     -149     
- Partials        4      117     +113     

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Thanks a lot!

1. Opening the subscribe window calls /api/v1/telegram/subscribe?site=SITE every time, which should be done only once after the page load. There is no need to store the results after the page reload, but within a single page load, it doesn't make sense to call it more than once.
2. After subscription and closing the subscription window I can't unsubscribe, the interface tries to subscribe me again unsuccessfully:
   
   Please check how email subscription knows that the user is already subscribed.

@paskal

Opening the subscribe window calls /api/v1/telegram/subscribe?site=SITE every time, which should be done only once after the page load. There is no need to store the results after the page reload, but within a single page load, it doesn't make sense to call it more than once.

Fixed. @Mavrin @akellbl4 FYI I've used a new custom useSessionStorage hook to save API call instead of redux which looks like project's standard, because I do not believe we need this state at other components of the application -> I do not believe global state should be used here. As per Uncle Bob global state can be a source of bugs, so I try to avoid using it to avoid application level side-effects. But please tell me if I need to use redux, I can refactor the code

After subscription and closing the subscription window I can't unsubscribe, the interface tries to subscribe me again unsuccessfully

Thanks for the bug. I did test the use-case where user refreshes the page, but did not test when user tries to unsubscribe immediately. A new integration test is created to cover this use-case and bug is fixed.

@paskal Another weird stuff: when I was testing yesterday I was receiving { "code": 17, "error": "already subscribed", ... } from GET /telegram/subscribe in case of user is already subscribed.

Today I receive 409 status code with empty response from the same endpoint for the same use-case.

Which is weird, because I test it on my remote deployment of remark42, which was not changed since yesterday. Anyway, I've covered this additional response in the spec file and implemented it the same as { "code": 17, "error": "already subscribed", ... }

@Mavrin @akellbl4 And what about i18n? I see that if I run pnpm run translation-check && pnpm run translation-check I have json files regenerated.

Shall I do it in this PR or it is a separate process? Can't find it in the project readme.

Also, do I need to extract common sentences to some common namespace? Like common.unsubscribe instead of subscribeByTelegram.unsubscribe and subscribeByEmail.unsubscribe?

@Mavrin @paskal @akellbl4 @umputun just pinging in case this PR has been fallen into cracks

@Mavrin thank you for your comments, everything is fixed

@Mavrin It has happened because of size check using npm install eventhough pnpm install used already in the same container. I've created a potential fix to skip npm install, but this fix requires your approval to be ran on CI to check if it helps

@goooseman could you pls rebase the PR to make a reasonable number of logically-consistent commits pls? Squashing it on my side will make a single one and I'm not sure if this is a good idea

@umputun done, but need re-apporval of CI files to run pipeline