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 QR API endpoint for Telegram auth and notifications #1246

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Conversation

paskal
Copy link
Sponsor Collaborator

@paskal paskal commented Jan 31, 2022

Telegram authentication requires you to open a chat on the phone. It's convenient to have a QR code for the case when you want to log in on the computer but have Telegram only on your phone and would be able to scan the QR instead of copy-pasting the link from the computer to the phone any other way. Generating QR codes restricted to strings starting with https://t.me/

Pre-requisite for #830 and #707 as in #1235 we discovered that it was discovered that generating QR on the frontend is rather expensive in terms of transferred data and increasing amount of JS code in the project, so we would just generate the QR and transfer on the backend. Server-side rendering is the future.

@paskal paskal force-pushed the paskal/tg_qr branch 2 times, most recently from a49d17a to a6f1d6b Compare January 31, 2022 18:47
@paskal paskal requested a review from umputun January 31, 2022 18:50
@paskal paskal added this to the v1.9 milestone Jan 31, 2022
Telegram authentication requires you to open a chat on the phone.
It's convenient to have a QR code for the case when you want to
log in on the computer but have Telegram only on your phone
and would be able to scan the QR instead of copy-pasting the link
from the computer to the phone any other way.

Originally we thought of generating QR on the client but found
backend-generated QR a better alternative because we avoid adding
one more JavaScript dependency to the frontend that way.
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

LGTM

@umputun umputun merged commit 603deca into master Jan 31, 2022
@umputun umputun deleted the paskal/tg_qr branch January 31, 2022 20:28
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1774749351

  • 18 of 20 (90.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 84.563%

Changes Missing Coverage Covered Lines Changed/Added Lines %
backend/app/rest/api/rest_public.go 17 19 89.47%
Totals Coverage Status
Change from base Build 1774739111: 0.04%
Covered Lines: 6075
Relevant Lines: 7184

💛 - Coveralls

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

Successfully merging this pull request may close these issues.

None yet

3 participants