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

feat: #1848 filter app lists when using the marketplace in Developer Edition #1851

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions packages/cognito-auth/src/__mocks__/cognito-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const mockLoginSession: LoginSession = {
userCode: 'SOME_USER_CODE',
isAdmin: false,
userTel: '123',
groups: [],
},
}

Expand Down
1 change: 1 addition & 0 deletions packages/cognito-auth/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface LoginIdentity {
userCode: string | null
isAdmin?: boolean
userTel: string
groups: string[]
}

export interface LoginSession {
Expand Down
2 changes: 2 additions & 0 deletions packages/cognito-auth/src/utils/cognito.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ describe('Session utils', () => {
name: 'SOME_NAME',
userCode: 'SOME_USER_CODE',
isAdmin: true,
groups: ['CustomerAdministrators'],
})
})

Expand All @@ -192,6 +193,7 @@ describe('Session utils', () => {
name: undefined,
userCode: null,
isAdmin: false,
groups: [],
})
})
})
Expand Down
1 change: 1 addition & 0 deletions packages/cognito-auth/src/utils/cognito.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export const deserializeIdToken = (loginSession: Partial<LoginSession> | undefin
userCode: decoded['custom:reapit:userCode'] || null,
isAdmin: Boolean(decoded['cognito:groups']?.includes('CustomerAdministrators')),
userTel: decoded['phone_number'],
groups: decoded['cognito:groups'] || [],
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const mockContext = {
adminId: 'mockAdminID',
userCode: 'mockUserCode',
userTel: 'mockTel',
groups: [],
},
loginType: 'CLIENT',
cognitoClientId: '123',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe('DeveloperAppRevisionModal', () => {
userCode: 'mockUserCode',
isAdmin: false,
userTel: '123',
groups: [],
}
const fn = handleCancelPendingRevisionsButtonClick(spyDispatch, appId, appRevisionId, loginIdentity)
fn()
Expand Down
2 changes: 2 additions & 0 deletions packages/marketplace/src/constants/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ export const COGNITO_HEADERS = {
} as StringMap

export const SANDBOX_CLIENT_ID = 'SBOX'

export const COGNITO_GROUP_DEVELOPER_EDITION = 'AgencyCloudDeveloperEdition'
1 change: 1 addition & 0 deletions packages/marketplace/src/core/__mocks__/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const mockLoginSession: LoginSession = {
userCode: 'SOME_USER_CODE',
isAdmin: false,
userTel: '123',
groups: ['AgencyCloudDeveloperEdition'],
Copy link
Contributor

Choose a reason for hiding this comment

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

should use const COGNITO_GROUP_DEVELOPER_EDITION

},
}

Expand Down
1 change: 1 addition & 0 deletions packages/marketplace/src/reducers/__stubs__/app-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const appState: ReduxState = {
adminId: null,
userCode: 'testUserCode',
userTel: '123',
groups: ['AgencyCloudDeveloperEdition'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

},
},
isTermAccepted: false,
Expand Down
8 changes: 6 additions & 2 deletions packages/marketplace/src/sagas/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { APPS_PER_PAGE } from '@/constants/paginator'
import { Action } from '@/types/core'
import { errorThrownServer } from '@/actions/error'
import errorMessages from '@/constants/error-messages'
import { selectClientId, selectFeaturedApps } from '@/selector/client'
import { selectClientId, selectFeaturedApps, selectDeveloperId } from '@/selector/client'
import { selectCategories } from '@/selector/app-categories'
import {
PagedResultCategoryModel_,
Expand All @@ -25,6 +25,7 @@ jest.mock('@reapit/elements')

const params = { data: { page: 1, search: 'app1', category: 'category1', searchBy: 'appName' } }
const clientId = 'DXX'
const developerId = '1234'

describe('clientDataFetch', () => {
const gen = cloneableGenerator(clientDataFetch as any)(params)
Expand All @@ -34,11 +35,14 @@ describe('clientDataFetch', () => {
expect(clone.next().value).toEqual(select(selectClientId))
expect(clone.next(clientId).value).toEqual(select(selectCategories))
expect(clone.next(appCategorieStub.data).value).toEqual(select(selectFeaturedApps))
expect(clone.next(featuredAppsDataStub.data).value).toEqual(select(selectDeveloperId))

const response = [appsDataStub.data, featuredAppsDataStub.data, appCategorieStub]
expect(clone.next(featuredAppsDataStub.data).value).toEqual(
expect(clone.next(developerId).value).toEqual(
all([
call(fetchAppsList, {
clientId,
developerId: [developerId],
category: params.data.category as any,
appName: params.data.search,
pageNumber: params.data.page,
Expand Down
7 changes: 5 additions & 2 deletions packages/marketplace/src/sagas/__tests__/installed-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Action } from '@/types/core'
import { cloneableGenerator } from '@redux-saga/testing-utils'
import { errorThrownServer } from '@/actions/error'
import errorMessages from '@/constants/error-messages'
import { selectClientId } from '@/selector/client'
import { selectClientId, selectDeveloperId } from '@/selector/client'
import { fetchAppsList } from '@/services/apps'
import { INSTALLED_APPS_PERPAGE } from '@/constants/paginator'

Expand All @@ -23,12 +23,15 @@ const params = { data: 1 }
describe('installed-apps fetch data', () => {
const gen = cloneableGenerator(installedAppsDataFetch)(params)
const clientId = 'DAC'
const developerId = '1234'

expect(gen.next().value).toEqual(put(installedAppsLoading(true)))
expect(gen.next().value).toEqual(select(selectClientId))
expect(gen.next(clientId).value).toEqual(
expect(gen.next(clientId).value).toEqual(select(selectDeveloperId))
expect(gen.next(developerId).value).toEqual(
call(fetchAppsList, {
clientId,
developerId: [developerId],
pageNumber: params.data,
pageSize: INSTALLED_APPS_PERPAGE,
onlyInstalled: true,
Expand Down
14 changes: 11 additions & 3 deletions packages/marketplace/src/sagas/__tests__/my-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Action } from '@/types/core'
import { cloneableGenerator } from '@redux-saga/testing-utils'
import { APPS_PER_PAGE } from '@/constants/paginator'
import errorMessages from '@/constants/error-messages'
import { selectClientId } from '@/selector/client'
import { selectClientId, selectDeveloperId } from '@/selector/client'
import { errorThrownServer } from '@/actions/error'
import { fetchAppsList } from '@/services/apps'

Expand All @@ -18,11 +18,19 @@ const params = { data: 1 }
describe('my-apps fetch data', () => {
const gen = cloneableGenerator(myAppsDataFetch)(params)
const clientId = 'DAC'
const developerId = '1234'

expect(gen.next().value).toEqual(put(myAppsLoading(true)))
expect(gen.next().value).toEqual(select(selectClientId))
expect(gen.next(clientId).value).toEqual(
call(fetchAppsList, { clientId, onlyInstalled: true, pageNumber: params.data, pageSize: APPS_PER_PAGE }),
expect(gen.next(clientId).value).toEqual(select(selectDeveloperId))
expect(gen.next(developerId).value).toEqual(
call(fetchAppsList, {
clientId,
developerId: [developerId],
onlyInstalled: true,
pageNumber: params.data,
pageSize: APPS_PER_PAGE,
}),
)

test('api call success', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/marketplace/src/sagas/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { errorThrownServer } from '../actions/error'
import errorMessages from '../constants/error-messages'
import { APPS_PER_PAGE, FEATURED_APPS } from '@/constants/paginator'
import { Action } from '@/types/core'
import { selectClientId, selectFeaturedApps } from '@/selector/client'
import { selectClientId, selectFeaturedApps, selectDeveloperId } from '@/selector/client'
import { selectCategories } from '@/selector/app-categories'
import { ClientAppSummary, ClientAppSummaryParams } from '@/reducers/client/app-summary'
import { logger } from '@reapit/utils'
Expand All @@ -24,6 +24,7 @@ export const clientDataFetch = function*({ data }) {
}
const currentCategories = yield select(selectCategories)
const currentFeaturedApps = yield select(selectFeaturedApps)
const developerId = yield select(selectDeveloperId)

// because the https://dev.platformmarketplace.reapit.net/categories endpoint does not return a filter for Direct API so
// we will have to manually check it here
Expand All @@ -33,6 +34,7 @@ export const clientDataFetch = function*({ data }) {
const [apps, featuredApps, categories] = yield all([
call(fetchAppsList, {
clientId,
developerId: developerId ? [developerId] : [],
category: isFilteringForDirectApiApps ? undefined : category,
[searchBy]: search,
pageNumber: page,
Expand Down
5 changes: 3 additions & 2 deletions packages/marketplace/src/sagas/installed-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { errorThrownServer } from '../actions/error'
import errorMessages from '../constants/error-messages'
import { Action } from '@/types/core'
import { INSTALLED_APPS_PERPAGE } from '@/constants/paginator'
import { selectClientId } from '@/selector/client'
import { selectClientId, selectDeveloperId } from '@/selector/client'
import { logger } from '@reapit/utils'
import { fetchAppsList } from '@/services/apps'

Expand All @@ -21,9 +21,10 @@ export const installedAppsDataFetch = function*({ data: page }) {
if (!clientId) {
return
}

const developerId = yield select(selectDeveloperId)
const response = yield call(fetchAppsList, {
clientId,
developerId: developerId ? [developerId] : [],
pageNumber: page,
pageSize: INSTALLED_APPS_PERPAGE,
onlyInstalled: true,
Expand Down
5 changes: 3 additions & 2 deletions packages/marketplace/src/sagas/my-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { errorThrownServer } from '../actions/error'
import errorMessages from '../constants/error-messages'
import { Action } from '@/types/core'
import { APPS_PER_PAGE } from '@/constants/paginator'
import { selectClientId } from '@/selector/client'
import { selectClientId, selectDeveloperId } from '@/selector/client'
import { logger } from '@reapit/utils'
import { fetchAppsList } from '@/services/apps'

Expand All @@ -17,9 +17,10 @@ export const myAppsDataFetch = function*({ data: page }) {
if (!clientId) {
return
}

const developerId = yield select(selectDeveloperId)
const response = yield call(fetchAppsList, {
clientId,
developerId: developerId ? [developerId] : [],
onlyInstalled: true,
pageNumber: page,
pageSize: APPS_PER_PAGE,
Expand Down
37 changes: 37 additions & 0 deletions packages/marketplace/src/selector/__tests__/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ReduxState } from '@/types/core'
import {
selectClientId,
selectDeveloperId,
selectLoggedUserEmail,
selectFeaturedApps,
selectWebComponentOpen,
Expand Down Expand Up @@ -37,6 +38,42 @@ describe('selectClientId', () => {
})
})

describe('selectDeveloperId', () => {
it('should run correctly when user in AgencyCloudDeveloperEdition group', () => {
const input = {
auth: {
loginSession: {
loginIdentity: {
clientId: '123',
email: '[email protected]',
developerId: '1234',
groups: ['AgencyCloudDeveloperEdition'],
},
},
},
} as ReduxState
const result = selectDeveloperId(input)
expect(result).toEqual('1234')
})

it('should run correctly when user NOT in AgencyCloudDeveloperEdition group', () => {
const input = {
auth: {
loginSession: {
loginIdentity: {
clientId: '123',
email: '[email protected]',
developerId: '1234',
groups: [''],
},
},
},
} as ReduxState
const result = selectDeveloperId(input)
expect(result).toEqual(null)
})
})

describe('selectLoggedUserEmail', () => {
it('should run correctly', () => {
const input = {
Expand Down
15 changes: 15 additions & 0 deletions packages/marketplace/src/selector/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,26 @@ import { WebComponentConfigResult } from '@/services/web-component'
import { InstalledAppsState } from '@/reducers/installed-apps'
import { AppSummaryModel } from '@reapit/foundations-ts-definitions'
import { ClientAppSummaryState } from '@/reducers/client/app-summary'
import { selectLoginIdentity } from '@/selector/auth'
import { COGNITO_GROUP_DEVELOPER_EDITION } from '@/constants/api'

export const selectClientId = (state: ReduxState) => {
return state?.auth?.loginSession?.loginIdentity?.clientId || ''
}

/**
* Need get developer id to filter apps list if this user belong to
* AgencyCloudDeveloperEdition group, if not just return null
* refer to this ticket https://github.com/reapit/foundations/issues/1848
*/
export const selectDeveloperId = (state: ReduxState) => {
Copy link
Contributor

@phmngocnghia phmngocnghia Jun 26, 2020

Choose a reason for hiding this comment

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

Naming seems ambiguous to me even with comment on the func. Imagine we will need to selectDeveloperId in the future without this logic. Something like selectDeveloperIdIfBelong(group) would be better. Just a suggestion

Copy link
Contributor

Choose a reason for hiding this comment

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

@phmngocnghia's comment makes sense, and should add return value type too.
It may cause confusion when we splitting to Client/Dev portal next week

Copy link
Contributor

Choose a reason for hiding this comment

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

selectDeveloperEditionId as a name maybe?

const loginIdentity = selectLoginIdentity(state)
if (loginIdentity?.groups.includes(COGNITO_GROUP_DEVELOPER_EDITION)) {
return state?.auth?.loginSession?.loginIdentity?.developerId || ''
}
return null
}

export const selectLoggedUserEmail = (state: ReduxState): string => {
return state?.auth?.loginSession?.loginIdentity?.email || ''
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const mockContext = {
adminId: 'mockAdminID',
userCode: 'mockUserCode',
userTel: 'mockUserTel',
groups: [],
},
loginType: 'CLIENT',
cognitoClientId: 'mockCognitoClientId',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const mockContext = {
adminId: 'mockAdminID',
userCode: 'mockUserCode',
userTel: '0799999999',
groups: [],
},
loginType: 'CLIENT',
cognitoClientId: '123',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const mockContext = {
adminId: 'mockAdminID',
userCode: 'mockUserCode',
userTel: 'mockTel',
groups: [],
},
loginType: 'CLIENT',
cognitoClientId: 'mockCognitoClientId',
Expand Down