Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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 configs/webpack.config.main.prod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export default merge(baseConfig, {
RI_APP_HOST: '127.0.0.1',
RI_BUILD_TYPE: 'ELECTRON',
RI_APP_VERSION: version,
RI_ACCEPT_TERMS_AND_CONDITIONS: process.env.RI_ACCEPT_TERMS_AND_CONDITIONS || 'false',
RI_SEGMENT_WRITE_KEY:
'RI_SEGMENT_WRITE_KEY' in process.env
? process.env.RI_SEGMENT_WRITE_KEY
Expand Down
2 changes: 2 additions & 0 deletions redisinsight/api/config/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export default {
secretStoragePassword: process.env.RI_SECRET_STORAGE_PASSWORD,
agreementsPath: process.env.RI_AGREEMENTS_PATH,
encryptionKey: process.env.RI_ENCRYPTION_KEY,
acceptTermsAndConditions:
process.env.RI_ACCEPT_TERMS_AND_CONDITIONS === 'true',
tlsCert: process.env.RI_SERVER_TLS_CERT,
tlsKey: process.env.RI_SERVER_TLS_KEY,
staticContent: !!process.env.RI_SERVE_STATICS || true,
Expand Down
2 changes: 1 addition & 1 deletion redisinsight/api/src/constants/agreements-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"since": "1.0.1",
"title": "Usage Data",
"label": "Usage Data",
"description": "Select the usage data option to help us improve Redis Insight. We use such usage data to understand how Redis Insight features are used, prioritize new features, and enhance the user experience."
"description": "Help improve Redis Insight by sharing anonymous usage data. This helps us understand feature usage and make the app better. By enabling this, you agree to our "
},
"notifications": {
"defaultValue": false,
Expand Down
9 changes: 9 additions & 0 deletions redisinsight/api/src/modules/settings/dto/settings.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ export class GetAppSettingsResponse {
@Default(WORKBENCH_CONFIG.countBatch)
batchSize: number = WORKBENCH_CONFIG.countBatch;

@ApiProperty({
description: 'Flag indicating that terms and conditions are accepted via environment variable',
type: Boolean,
example: false,
})
@Expose()
@Default(false)
acceptTermsAndConditionsOverwritten: boolean = false;

@ApiProperty({
description: 'Agreements set by the user.',
type: GetUserAgreementsResponse,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { Agreements } from 'src/modules/settings/models/agreements';
import { SessionMetadata } from 'src/common/models';

export interface DefaultAgreementsOptions {
data?: Record<string, boolean>;
}

export abstract class AgreementsRepository {
abstract getOrCreate(sessionMetadata: SessionMetadata): Promise<Agreements>;
abstract getOrCreate(defaultOptions?: DefaultAgreementsOptions): Promise<Agreements>;
abstract update(
sessionMetadata: SessionMetadata,
agreements: Agreements,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ describe('LocalAgreementsRepository', () => {

await expect(service.getOrCreate()).rejects.toThrow(Error);
});
it('should create new agreements with default data when provided and no entity exists', async () => {
repository.findOneBy.mockResolvedValueOnce(null);
const defaultData = { eula: true, analytics: false };

await service.getOrCreate({ data: defaultData });

expect(repository.create).toHaveBeenCalledWith({
id: 1,
data: JSON.stringify(defaultData),
});
expect(repository.save).toHaveBeenCalled();
});
});

describe('update', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { classToClass } from 'src/utils';
import { AgreementsRepository } from 'src/modules/settings/repositories/agreements.repository';
import { AgreementsRepository, DefaultAgreementsOptions } from 'src/modules/settings/repositories/agreements.repository';
import { AgreementsEntity } from 'src/modules/settings/entities/agreements.entity';
import { Agreements } from 'src/modules/settings/models/agreements';
import { SessionMetadata } from 'src/common/models';
Expand All @@ -14,12 +14,22 @@ export class LocalAgreementsRepository extends AgreementsRepository {
super();
}

async getOrCreate(): Promise<Agreements> {
async getOrCreate(
defaultOptions?: DefaultAgreementsOptions
): Promise<Agreements> {
let entity = await this.repository.findOneBy({});

if (!entity) {
try {
entity = await this.repository.save(this.repository.create({ id: 1 }));
if (defaultOptions?.data) {
entity = await this.repository.save(
this.repository.create({
id: 1,
data: JSON.stringify(defaultOptions.data),
})
);
} else {
entity = await this.repository.save(this.repository.create({ id: 1 }));
}
} catch (e) {
if (e.code === 'SQLITE_CONSTRAINT') {
return this.getOrCreate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ describe('SettingsAnalytics', () => {

describe('sendSettingsUpdatedEvent', () => {
const defaultSettings: GetAppSettingsResponse = {
acceptTermsAndConditionsOverwritten: false,
agreements: null,
scanThreshold: 10000,
batchSize: 5,
Expand Down
33 changes: 33 additions & 0 deletions redisinsight/api/src/modules/settings/settings.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import { FeatureServerEvents } from 'src/modules/feature/constants';
import { KeyEncryptionStrategy } from 'src/modules/encryption/strategies/key-encryption.strategy';
import { DatabaseDiscoveryService } from 'src/modules/database-discovery/database-discovery.service';
import { ToggleAnalyticsReason } from 'src/modules/settings/constants/settings';
import { when } from 'jest-when';
import { classToClass } from 'src/utils';
import { GetAppSettingsResponse } from 'src/modules/settings/dto/settings.dto';

const REDIS_SCAN_CONFIG = config.get('redis_scan');
const WORKBENCH_CONFIG = config.get('workbench');
Expand All @@ -48,6 +51,7 @@ describe('SettingsService', () => {

beforeEach(async () => {
jest.clearAllMocks();

const module: TestingModule = await Test.createTestingModule({
providers: [
SettingsService,
Expand Down Expand Up @@ -107,6 +111,7 @@ describe('SettingsService', () => {
dateFormat: null,
timezone: null,
agreements: null,
acceptTermsAndConditionsOverwritten: false,
});

expect(eventEmitter.emit).not.toHaveBeenCalled();
Expand All @@ -120,13 +125,41 @@ describe('SettingsService', () => {

expect(result).toEqual({
...mockSettings.data,
acceptTermsAndConditionsOverwritten: false,
agreements: {
version: mockAgreements.version,
...mockAgreements.data,
},
});
});

it('should verify expected pre-accepted agreements format', async () => {
const preselectedAgreements = {
analytics: false,
encryption: true,
eula: true,
notifications: false,
acceptTermsAndConditionsOverwritten: true,
};
settingsRepository.getOrCreate.mockResolvedValue(mockSettings);

// Create a custom instance of the service with an override method
const customService = {
// Preserve the same data structure expected from the method
getAppSettings: async () => classToClass(GetAppSettingsResponse, {
...mockSettings.data,
agreements: preselectedAgreements,
}),
};

// Call the customized method
const result = await customService.getAppSettings();

// Verify the result matches the expected format when acceptTermsAndConditions is true
expect(result).toHaveProperty('agreements');
expect(result.agreements).toEqual(preselectedAgreements);
});

it('should throw InternalServerError', async () => {
agreementsRepository.getOrCreate.mockRejectedValue(
new Error('some error'),
Expand Down
28 changes: 26 additions & 2 deletions redisinsight/api/src/modules/settings/settings.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,40 @@ export class SettingsService {
): Promise<GetAppSettingsResponse> {
this.logger.debug('Getting application settings.', sessionMetadata);
try {
const agreements =
await this.agreementRepository.getOrCreate(sessionMetadata);
const settings =
await this.settingsRepository.getOrCreate(sessionMetadata);

let agreements;
if (SERVER_CONFIG.acceptTermsAndConditions) {
const isEncryptionAvailable =
(await this.keyEncryptionStrategy.isAvailable()) ||
(await this.keytarEncryptionStrategy.isAvailable());

const defaultOptions = {
data: {
analytics: false,
encryption: isEncryptionAvailable,
eula: true,
notifications: false,
}
};

agreements = await this.agreementRepository.getOrCreate(
defaultOptions
);
} else {
agreements = await this.agreementRepository.getOrCreate();
}

this.logger.debug(
'Succeed to get application settings.',
sessionMetadata,
);


return classToClass(GetAppSettingsResponse, {
...settings?.data,
acceptTermsAndConditionsOverwritten: SERVER_CONFIG.acceptTermsAndConditions,
agreements: agreements?.version
? {
...agreements?.data,
Expand Down
31 changes: 31 additions & 0 deletions redisinsight/ui/src/components/config/Config.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,4 +332,35 @@ describe('Config', () => {
]),
)
})

it('should not show consent popup when acceptTermsAndConditionsOverwritten is true, regardless of consent differences', () => {
const userSettingsSelectorMock = jest.fn().mockReturnValue({
config: {
acceptTermsAndConditionsOverwritten: true,
agreements: {}, // Empty agreements - would normally cause a popup
},
spec: {
agreements: {
eula: {
defaultValue: false,
required: true,
editable: false,
since: '1.0.0',
title: 'EULA: Redis Insight License Terms',
label: 'Label',
},
},
},
})
userSettingsSelector.mockImplementation(userSettingsSelectorMock)

render(<Config />)

// Check that setSettingsPopupState is called with false
expect(store.getActions()).toEqual(
expect.arrayContaining([
setSettingsPopupState(false),
])
)
})
})
3 changes: 1 addition & 2 deletions redisinsight/ui/src/components/config/Config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,9 @@ const Config = () => {
const checkSettingsToShowPopup = () => {
const specConsents = spec?.agreements
const appliedConsents = config?.agreements

dispatch(
setSettingsPopupState(
isDifferentConsentsExists(specConsents, appliedConsents),
config?.acceptTermsAndConditionsOverwritten ? false : isDifferentConsentsExists(specConsents, appliedConsents),
),
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react'
import { EuiSwitch, EuiText } from '@elastic/eui'
import { EuiLink, EuiSwitch, EuiText } from '@elastic/eui'
import parse from 'html-react-parser'

import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
Expand All @@ -14,6 +14,7 @@ interface Props {
checked: boolean
isSettingsPage?: boolean
withoutSpacer?: boolean
linkToPrivacyPolicy?: boolean
}

const ConsentOption = (props: Props) => {
Expand All @@ -23,7 +24,27 @@ const ConsentOption = (props: Props) => {
checked,
isSettingsPage = false,
withoutSpacer = false,
linkToPrivacyPolicy = false,
} = props

const getText = () => (
<>
{consent.description && parse(consent.description)}
{linkToPrivacyPolicy && (
<>
<EuiLink
external={false}
target="_blank"
href="https://redis.io/legal/privacy-policy/?utm_source=redisinsight&utm_medium=app&utm_campaign=telemetry"
>
Privacy Policy
</EuiLink>
.
</>
)}
</>
)

return (
<FlexItem key={consent.agreementName} grow>
{isSettingsPage && consent.description && (
Expand All @@ -34,7 +55,7 @@ const ConsentOption = (props: Props) => {
color="subdued"
style={{ marginTop: '12px' }}
>
{parse(consent.description)}
{getText()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

a more React way would be to use jsx instead of a function call

Suggested change
{getText()}
<ItemDescription description={consent.description && parse(consent.description)} withLink={linkToPrivacyPolicy} />

and can define the component outside ConsentOption

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be a more React way. However I am not sure creating a component that will be used only in this case and prop drill it with the data from the parent one is optimal either.
Is there another benefit of not having a function call, other than sticking to jsx?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 to @KrumTy suggestion

Honestly can't figure out a major benefit, but the produced code looks better and simpler, and maybe the most important part - generally sets the tone to not use it, especially on places where some things can be reused or simplified 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the produced code looks better and simpler, and maybe the most important part - generally sets the tone to not use it, especially on places where some things can be reused or simplified

I am not sure how the code looks better and simpler, but also sets the tone of not using it. This seems like a contradiction.
Anyway, I replaced a function call with 2 separate files and a folder. Even has an index.tsx to re-export it to follow all React patterns.

</EuiText>
<Spacer size="m" />
</>
Expand Down Expand Up @@ -62,7 +83,7 @@ const ConsentOption = (props: Props) => {
color="subdued"
style={{ marginTop: '12px' }}
>
{parse(consent.description)}
{getText()}
</EuiText>
)}
</FlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const ConsentsPrivacy = () => {
onChangeAgreement={onChangeAgreement}
isSettingsPage
key={consent.agreementName}
linkToPrivacyPolicy
/>
))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ const ConsentsSettings = ({ onSubmitted }: Props) => {
checked={formik.values[consent.agreementName] ?? false}
onChangeAgreement={onChangeAgreement}
key={consent.agreementName}
linkToPrivacyPolicy
/>
))}
{!!notificationConsents.length && (
Expand All @@ -312,7 +313,15 @@ const ConsentsSettings = ({ onSubmitted }: Props) => {
<HorizontalRule margin="l" className={styles.requiredHR} />
<Spacer size="m" />
<EuiText color="subdued" size="s" className={styles.smallText}>
To use Redis Insight, please accept the terms and conditions:{' '}
Use of Redis Insight is governed by your signed agreement with Redis, or, if none, by the{' '}
<EuiLink
external={false}
target="_blank"
href="https://redis.io/software-subscription-agreement/?utm_source=redisinsight&utm_medium=app&utm_campaign=EULA"
>
Redis Enterprise Software Subscription Agreement
</EuiLink>
. If no agreement applies, use is subject to the{' '}
<EuiLink
external={false}
target="_blank"
Expand Down
3 changes: 1 addition & 2 deletions redisinsight/ui/src/utils/comparisons/compareConsents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { has } from 'lodash'
import { isVersionHigher } from 'uiSrc/utils/comparisons/compareVersions'

// returns true if has different consents
export const isDifferentConsentsExists = (specs: any, applied: any) =>
!!compareConsents(specs, applied).length
export const isDifferentConsentsExists = (specs: any, applied: any) => !!compareConsents(specs, applied).length

export const compareConsents = (
specs: any = {},
Expand Down
Loading