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

Cleanup feedback related state, old LIFF page and token mechanism #315

Merged
merged 3 commits into from Jun 30, 2022

Conversation

MrOrz
Copy link
Member

@MrOrz MrOrz commented Jun 25, 2022

  • Remove feedback/yes and feedback/no LIFF pages
  • Remove LINE bot GraphQL endpoints related to above pages - context query and voteReply mutation
  • Remove LIFF <> LINE bot graphql's bearer token mechanism
    • It was introduced in LIFF as token URL param. LIFF forwards the token back in LINE bot GraphQL, so that LINE bot graphql can authenticate user when determining GraphQL context.
    • However, this whole mechanism is replaced by LIFF v2's ID token (or possibly access token if we remove ID token permission). (Disucssed in 2021/4/13)
    • The last place we use such token was in PositiveFeedback and NegativeFeedback LIFF pages.
    • The mechanism can be removed after the aforementioned LIFF pages are removed
  • Remove askingReplyFeedback state handler and its related prefix logic

- It was introduced so that LIFF forwards the token back in GraphQL to authenticate user
- However, this whole mechanism is replaced by LIFF v2's ID token (or possibly access token if we remove ID token permission)
- The last place we use such token was in PositiveFeedback and NegativeFeedback LIFF pages.
- The mechanism can be removed after the aforementioned LIFF pages are removed
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2559819518

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-1.3%) to 85.648%

Files with Coverage Reduction New Missed Lines %
src/graphql/index.js 1 86.96%
Totals Coverage Status
Change from base Build 2559635494: -1.3%
Covered Lines: 889
Relevant Lines: 1017

💛 - Coveralls

@MrOrz MrOrz self-assigned this Jun 25, 2022
@MrOrz MrOrz requested review from nonumpa and bil4444 June 25, 2022 08:49
@MrOrz
Copy link
Member Author

MrOrz commented Jun 25, 2022

This branch has been deployed to staging LINE bot; everyone can give it a try.

Base automatically changed from feedback-revamp to master June 30, 2022 16:06
@MrOrz MrOrz merged commit dd0a007 into master Jun 30, 2022
@MrOrz MrOrz deleted the cleanup-state branch June 30, 2022 16:12
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.

None yet

3 participants