Skip to content

fix(android): remove current view update delay #1080

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

Merged
merged 4 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- Bump Instabug iOS SDK to v12.5.0 ([#1085](https://github.com/Instabug/Instabug-React-Native/pull/1085)). [See release notes](https://github.com/instabug/instabug-ios/releases/tag/12.5.0).
- Bump Instabug Android SDK to v12.5.1 ([#1088](https://github.com/Instabug/Instabug-React-Native/pull/1085)). See release notes for [v12.5.0](https://github.com/Instabug/android/releases/tag/v12.5.0) and [v12.5.1](https://github.com/Instabug/android/releases/tag/v12.5.1).

### Fixed

- Fix a delay issue in reporting the 'Current View' that resulted in displaying outdated values ([#1080](https://github.com/Instabug/Instabug-React-Native/pull/1080)).

## [12.4.0](https://github.com/Instabug/Instabug-React-Native/compare/v12.2.0...v12.4.0) (December 7, 2023)

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,29 @@ public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) {
});
}

/**
* Reports that the screen name been changed (Current View).
*
* @param screenName string containing the screen name
*
*/
@ReactMethod
public void reportCurrentViewChange(final String screenName) {
MainThreadHandler.runOnMainThread(new Runnable() {
@Override
public void run() {
try {
Method method = getMethod(Class.forName("com.instabug.library.Instabug"), "reportCurrentViewChange", String.class);
if (method != null) {
method.invoke(null, screenName);
}
} catch (Exception e) {
e.printStackTrace();
}
}
});
}

/**
* Reports that the screen has been changed (Repro Steps) the screen sent to this method will be the 'current view' on the dashboard
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,18 @@ public void tearDown() {
}
}

@Test
public void givenString$reportCurrentViewChange_whenQuery_thenShouldCallNativeApiWithString() throws Exception {
// when
rnModule.reportCurrentViewChange("screen");
Method privateStringMethod = getMethod(Class.forName("com.instabug.library.Instabug"), "reportCurrentViewChange", String.class);
privateStringMethod.setAccessible(true);

// then
verify(Instabug.class, VerificationModeFactory.times(1));
privateStringMethod.invoke("reportCurrentViewChange","screen");
}

@Test
public void givenString$reportScreenChange_whenQuery_thenShouldCallNativeApiWithString() throws Exception {
// when
Expand Down
20 changes: 20 additions & 0 deletions src/modules/Instabug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,23 @@ export const setEnabled = (isEnabled: boolean) => {
NativeInstabug.setEnabled(isEnabled);
};

/**
* Reports that the screen name been changed (Current View field on dashboard).
* only for android.
*
* Normally reportScreenChange handles taking a screenshot for reproduction
* steps and the Current View field on the dashboard. But we've faced issues
* in android where we needed to separate them, that's why we only call it
* for android.
*
* @param screenName string containing the screen name
*/
function reportCurrentViewForAndroid(screenName: string | null) {
if (Platform.OS === 'android' && screenName != null) {
NativeInstabug.reportCurrentViewChange(screenName);
}
}

/**
* Initializes the SDK.
* This is the main SDK method that does all the magic. This is the only
Expand All @@ -55,6 +72,7 @@ export const init = (config: InstabugConfig) => {
_isFirstScreen = true;
_currentScreen = firstScreen;

reportCurrentViewForAndroid(firstScreen);
setTimeout(() => {
if (_currentScreen === firstScreen) {
NativeInstabug.reportScreenChange(firstScreen);
Expand Down Expand Up @@ -458,6 +476,7 @@ export const onNavigationStateChange = (
const prevScreen = InstabugUtils.getActiveRouteName(prevState);

if (prevScreen !== currentScreen) {
reportCurrentViewForAndroid(currentScreen);
if (_currentScreen != null && _currentScreen !== firstScreen) {
NativeInstabug.reportScreenChange(_currentScreen);
_currentScreen = null;
Expand All @@ -478,6 +497,7 @@ export const onStateChange = (state?: NavigationStateV5) => {
}

const currentScreen = InstabugUtils.getFullRoute(state);
reportCurrentViewForAndroid(currentScreen);
if (_currentScreen !== null && _currentScreen !== firstScreen) {
NativeInstabug.reportScreenChange(_currentScreen);
_currentScreen = null;
Expand Down
1 change: 1 addition & 0 deletions src/native/NativeInstabug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface InstabugNativeModule extends NativeModule {
): void;
setTrackUserSteps(isEnabled: boolean): void;
reportScreenChange(firstScreen: string): void;
reportCurrentViewChange(screenName: string): void;
addPrivateView(nativeTag: number | null): void;
removePrivateView(nativeTag: number | null): void;

Expand Down
1 change: 1 addition & 0 deletions test/mocks/mockInstabug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const mockInstabug: InstabugNativeModule = {
show: jest.fn(),
setPreSendingHandler: jest.fn(),
reportScreenChange: jest.fn(),
reportCurrentViewChange: jest.fn(),
addExperiments: jest.fn(),
removeExperiments: jest.fn(),
clearAllExperiments: jest.fn(),
Expand Down
92 changes: 91 additions & 1 deletion test/modules/Instabug.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,33 @@ describe('Instabug Module', () => {
});
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('onNavigationStateChange should call the native method reportCurrentViewChange on Android Platform', async () => {
Platform.OS = 'android';
InstabugUtils.getActiveRouteName = jest.fn().mockImplementation((screenName) => screenName);

// @ts-ignore
Instabug.onNavigationStateChange('home', 'settings');

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).toBeCalledTimes(1);
expect(NativeInstabug.reportCurrentViewChange).toBeCalledWith('settings');
});
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('onNavigationStateChange should not call the native method reportCurrentViewChange on iOS Platform', async () => {
Platform.OS = 'ios';
InstabugUtils.getActiveRouteName = jest.fn().mockImplementation((screenName) => screenName);

// @ts-ignore
Instabug.onNavigationStateChange('home', 'settings');

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).not.toBeCalled();
});
});

it('onNavigationStateChange should not call the native method reportScreenChange if screen is the same', (done) => {
InstabugUtils.getActiveRouteName = jest.fn().mockImplementation((screenName) => screenName);

Expand All @@ -122,7 +149,20 @@ describe('Instabug Module', () => {
}, 1500);
});

it('onNavigationStateChange should call the native method reportScreenChange immediatly if _currentScreen is set', async () => {
it('onNavigationStateChange should not call the native method reportCurrentViewChange if screen is the same', (done) => {
InstabugUtils.getActiveRouteName = jest.fn().mockImplementation((screenName) => screenName);

// @ts-ignore
Instabug.onNavigationStateChange('home', 'home');

// Wait for 1.5s as reportScreenChange is delayed by 1s
setTimeout(() => {
expect(NativeInstabug.reportCurrentViewChange).not.toBeCalled();
done();
}, 1500);
});

it('onNavigationStateChange should call the native method reportScreenChange immediately if _currentScreen is set', async () => {
InstabugUtils.getActiveRouteName = jest.fn().mockImplementation((screenName) => screenName);

// sets _currentScreen and waits for 1s as _currentScreen is null
Expand Down Expand Up @@ -150,6 +190,31 @@ describe('Instabug Module', () => {
});
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('onStateChange should call the native method reportCurrentViewChange on Android Platform', async () => {
Platform.OS = 'android';
const state = { routes: [{ name: 'ScreenName' }], index: 0 };
// @ts-ignore
Instabug.onStateChange(state);

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).toBeCalledTimes(1);
expect(NativeInstabug.reportCurrentViewChange).toBeCalledWith('ScreenName');
});
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('onStateChange should not call the native method reportCurrentViewChange on iOS Platform', async () => {
Platform.OS = 'ios';
const state = { routes: [{ name: 'ScreenName' }], index: 0 };
// @ts-ignore
Instabug.onStateChange(state);

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).not.toBeCalled();
});
});

it('onStateChange should call the native method reportScreenChange immediately if _currentScreen is set', async () => {
// sets _currentScreen and waits for 1s as _currentScreen is null
const state = { routes: [{ name: 'ScreenName' }], index: 0 };
Expand Down Expand Up @@ -195,6 +260,31 @@ describe('Instabug Module', () => {
});
});

it('init should call reportCurrentViewChange on Android Platform', async () => {
Platform.OS = 'android';
Instabug.init({
token: 'some-token',
invocationEvents: [InvocationEvent.none],
});

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).toBeCalledTimes(1);
expect(NativeInstabug.reportCurrentViewChange).toBeCalledWith('Initial Screen');
});
});

it('init should not call reportCurrentViewChange on ios Platform', async () => {
Platform.OS = 'ios';
Instabug.init({
token: 'some-token',
invocationEvents: [InvocationEvent.none],
});

await waitForExpect(() => {
expect(NativeInstabug.reportCurrentViewChange).not.toBeCalled();
});
});

it('should call the native method setUserData', () => {
const userData = 'userData';
Instabug.setUserData(userData);
Expand Down