8000 Feat add flush method on Android by luancurti · Pull Request #64 · react-native-cookies/cookies · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feat add flush method on Android #64

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 24, 2020
Merged

Feat add flush method on Android #64

merged 3 commits into from
Jul 24, 2020

Conversation

luancurti
Copy link
Contributor
@luancurti luancurti commented Jul 22, 2020 •
edited
Loading

Summary

Add a method to flush cookies

Test Plan

Call the CookieManager.flush and the cookies should be flushed

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS ❌
Android ✅

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

@luancurti luancurti added the enhancement New feature or request label Jul 22, 2020
@luancurti luancurti requested a review from safaiyeh July 22, 2020 01:32
@luancurti luancurti self-assigned this Jul 22, 2020
@luancurti luancurti force-pushed the feat/add-flush-method branch from e3566c6 to fd78bdc Compare July 22, 2020 01:36
luancurti added 2 commits July 21, 2020 22:44
@luancurti
feat(flush): add flush method on android
56d6d24
@luancurti
chore(android): update min sdk to 21
d27a4fb 
- to use flush method we need to set the min sdk to 21 because flush is
  added in sdk 21
@luancurti luancurti force-pushed the feat/add-flush-method branch from fd78bdc to d27a4fb Compare July 22, 2020 01:44
@luancurti luancurti linked an issue Jul 22, 2020 that may be closed by this pull request
CookieManager flush needed #58
Closed
@luancurti luancurti changed the title Feat add flush method Feat add flush method on Android Jul 22, 2020
@luancurti luancurti marked this pull request as ready for review July 22, 2020 03:46
@luancurti luancurti force-pushed the feat/add-flush-method branch from 69554a9 to 310a998 Compare July 22, 2020 16:40
@safaiyeh
Copy link
Member

We call flush in varies methods, I'm assuming the motivation behind this PR is that there are cases where it did not call flush correctly.

This begs the question should the methods manage calling flush themselves or the developer should always explicitly call flush?

@luancurti
Copy link
Contributor Author
luancurti commented Jul 22, 2020 •
edited
Loading

We call flush in varies methods, I'm assuming the motivation behind this PR is that there are cases where it did not call flush correctly.

This begs the question should the methods manage calling flush themselves or the developer should always explicitly call flush?

@safaiyeh I don't know what the motivation I only expose this method because this issue @perottilds can you explain why do you need to call this method?

@de-perotti
Copy link
de-perotti commented Jul 24, 2020 •
edited
Loading

Sure, @luancurti

Below a method I had to use to workaround the fact that I can't directly flush the cookies when my page first loads.
Since flushing is not immediate (on Android), I save the token in async storage (yes it's not safe yada yada) and fetch it as soon as the user opens up the app again, since most interactions with the app have a short life span

import { LoginData } from '../observables/OnLoginObservable';
import RNCookieManager from '@react-native-community/cookies';
import AsyncStorage from '@react-native-community/async-storage';
import { config } from '../config';

const ACCESS_TOKEN_STORAGE_KEY = 'cookies.accessToken';

export class CookieManager {
  static async handleLogin(_loginData: LoginData | null) {
    const cookies = await RNCookieManager.get(config.DOMAIN);

    if (!cookies || !cookies.accessToken) {
      return;
    }

    await AsyncStorage.setItem(
      ACCESS_TOKEN_STORAGE_KEY,
      JSON.stringify(cookies.accessToken),
    );
  }

  static async setAccessTokenFromStorage() {
    const accessToken = await AsyncStorage.getItem(ACCESS_TOKEN_STORAGE_KEY);

    if (!accessToken) {
      return;
    }

    await RNCookieManager.set(config.DOMAIN, JSON.parse(accessToken));
  }
}

@de-perotti de-perotti mentioned this pull request Jul 24, 2020
Closed
@safaiyeh
Copy link
Member

That makes sense @perottilds thanks!

Appreciate the work on this @luancurti landing this.

@safaiyeh safaiyeh merged commit 74cd6a1 into master Jul 24, 2020
@safaiyeh safaiyeh deleted the feat/add-flush-method branch July 24, 2020 17:53
react-native-community-bot pushed a commit that referenced this pull request Jul 24, 2020
@semantic-release-bot
chore(release): 4.0.0 [skip ci]
ddf46ff 
# [4.0.0](v3.0.3...v4.0.0) (2020-07-24)

### Features

* **Android:** Add flush method on Android ([#64](#64)) ([74cd6a1](74cd6a1))

### BREAKING CHANGES

* **Android:** Android Minmum SDK bumped to API 21
@react-native-community-bot
Copy link
Collaborator

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@safaiyeh safaiyeh Awaiting requested review from safaiyeh

Assignees

@luancurti luancurti

Labels
enhancement New feature or request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CookieManager flush needed
4 participants
@luancurti @safaiyeh @de-perotti @react-native-community-bot
0