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

[RW-13480][risk=no] Workspace Banners for Initial Credits #8951

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
40bdb61
Added basic tests.
evrii Nov 12, 2024
cc7918c
Add expiration date.
evrii Nov 12, 2024
dee730b
Update styling
evrii Nov 12, 2024
e76615e
Add checks for extension button.
evrii Nov 12, 2024
a5ff2f3
Add extension and info buttons.
evrii Nov 12, 2024
0adb7e4
Merge remote-tracking branch 'origin/main' into eric/RW-13481
evrii Nov 14, 2024
ad412f5
Merge remote-tracking branch 'origin/main' into eric/RW-13481
evrii Nov 15, 2024
c214129
Added extension field.
evrii Nov 18, 2024
cd7f7d3
First pass at banners
evrii Nov 18, 2024
bdd7f49
Simplified logic?
evrii Nov 18, 2024
a2dbc9d
Added config values.
evrii Nov 18, 2024
42b159c
[RW-13735][risk=no] Added modal to trigger initial credit extension. …
evrii Nov 19, 2024
f50fb7b
Merge remote-tracking branch 'origin/eric/RW-13481' into eric/RW-13480
evrii Nov 19, 2024
bd12bb9
Fix title and eligibility usage
evrii Nov 19, 2024
7b35bd6
Merge remote-tracking branch 'origin/main' into eric/RW-13480
evrii Nov 19, 2024
6480a2f
[risk=low][no ticket] [dependabot] Bump cross-spawn from 7.0.3 to 7.0…
dependabot[bot] Nov 19, 2024
7881475
laucn multi-zone (#8956)
yonghaoy Nov 19, 2024
92a2784
remove getUploadResult in verifyAndLog (#8955)
Qi77Qi Nov 19, 2024
0abe313
Merge remote-tracking branch 'origin/main' into eric/RW-13480
evrii Nov 19, 2024
0f0326e
Update banner
evrii Nov 19, 2024
8092dba
Cleaner banner?
evrii Nov 19, 2024
e507305
Merge remote-tracking branch 'origin/main' into eric/RW-13480
evrii Nov 20, 2024
d7ec332
Cleanup
evrii Nov 20, 2024
34957ec
Add expiring soon banners.
evrii Nov 20, 2024
d1b1f87
Add expired but eligible tests
evrii Nov 20, 2024
f463eec
Add expired and ineligible tests
evrii Nov 20, 2024
73378ea
Hide banner if not appropriate
evrii Nov 20, 2024
3c286c7
Cleanup
evrii Nov 22, 2024
cbca190
Cleanup
evrii Nov 22, 2024
cd304ee
Updated language
evrii Nov 22, 2024
300240f
Updated spacing
evrii Nov 22, 2024
3c6ec5e
Updated test email.
evrii Nov 22, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ public interface WorkspaceMapper {
target = "initialCredits.expirationEpochMillis",
source = "dbWorkspace.creator",
qualifiedByName = "getInitialCreditsExpiration")
@Mapping(
target = "initialCredits.extensionEpochMillis",
source = "dbWorkspace.creator",
qualifiedByName = "getInitialCreditsExpiration")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this sets extension to the same value as expiration. Is this intentional?

@Mapping(target = "cdrVersionId", source = "dbWorkspace.cdrVersion")
@Mapping(target = "accessTierShortName", source = "dbWorkspace.cdrVersion.accessTier.shortName")
@Mapping(target = "googleProject", source = "dbWorkspace.googleProject")
Expand Down Expand Up @@ -128,6 +132,10 @@ default List<WorkspaceResponse> toApiWorkspaceResponseList(
target = "initialCredits.expirationEpochMillis",
source = "creator",
qualifiedByName = "getInitialCreditsExpiration")
@Mapping(
target = "initialCredits.extensionEpochMillis",
source = "creator",
qualifiedByName = "getInitialCreditsExpiration")
@Mapping(target = "initialCredits.exhausted", source = "dbWorkspace.initialCreditsExhausted")
@Mapping(target = "etag", source = "version", qualifiedByName = "versionToEtag")
@Mapping(
Expand Down
3 changes: 3 additions & 0 deletions api/src/main/resources/workbench-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13064,6 +13064,9 @@ components:
expirationEpochMillis:
type: integer
format: int64
extensionEpochMillis:
type: integer
format: int64
parameters:
userId:
name: userId
Expand Down
198 changes: 198 additions & 0 deletions ui/src/app/components/invalid-billing-banner.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
import '@testing-library/jest-dom';
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file is not in the same location as what it's testing. Please move one or the other.


import * as React from 'react';
import { MemoryRouter } from 'react-router-dom';

import { ProfileApi } from 'generated/fetch';

import { screen } from '@testing-library/dom';
import { render } from '@testing-library/react';
import { DuccSignatureState } from 'app/components/data-user-code-of-conduct';
import { InvalidBillingBanner } from 'app/pages/workspace/invalid-billing-banner';
import {
profileApi,
registerApiClient,
} from 'app/services/swagger-fetch-clients';
import { plusDays } from 'app/utils/dates';
import { currentWorkspaceStore } from 'app/utils/navigation';
import { profileStore, serverConfigStore } from 'app/utils/stores';

import defaultServerConfig from 'testing/default-server-config';
import {
ProfileApiStub,
ProfileStubVariables,
} from 'testing/stubs/profile-api-stub';
import { workspaceDataStub } from 'testing/stubs/workspaces';

describe('InvalidBillingBanner', () => {
const load = jest.fn();
const reload = jest.fn();
const updateCache = jest.fn();
const warningThresholdDays = 5; // arbitrary
const me = ProfileStubVariables.PROFILE_STUB.username;
const someOneElse = '[email protected]';

const component = (
signatureState: DuccSignatureState = DuccSignatureState.UNSIGNED
) =>
render(
<MemoryRouter>
<InvalidBillingBanner
{...{ signatureState }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't look right. None of these props are used by InvalidBillingBanner.

hideSpinner={() => {}}
showSpinner={() => {}}
/>
</MemoryRouter>
);

beforeEach(() => {
registerApiClient(ProfileApi, new ProfileApiStub());

// Do I need this?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think so

reload.mockImplementation(async () => {
const newProfile = await profileApi().getMe();
profileStore.set({ profile: newProfile, load, reload, updateCache });
});

profileStore.set({
profile: ProfileStubVariables.PROFILE_STUB,
load,
reload,
updateCache,
});
serverConfigStore.set({
config: {
...defaultServerConfig,
initialCreditsExpirationWarningDays: warningThresholdDays,
},
});
});

const setupWorkspace = (
exhausted: boolean,
expired: boolean,
expiringSoon: boolean,
ownedByMe: boolean
) => {
// Set expiration date to be in the past if expired, in the future if not.
// If expiring soon, set it to be within the warning threshold, and just outside otherwise.
// Expired and expiringSoon are mutually exclusive.
const daysUntilExpiration = expired
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this

? -1
: warningThresholdDays + (expiringSoon ? -1 : 1);
currentWorkspaceStore.next({
...workspaceDataStub,
initialCredits: {
exhausted,
expired,
expirationEpochMillis: plusDays(Date.now(), daysUntilExpiration),
},
creator: ownedByMe ? me : someOneElse,
});
};

/* All banners have "initial credits" in the text. Banner text can have one or more links in it.
* React Testing Library has a hard time finding text that is split across multiple elements
* (like text and links), so we can't use getByText to find the banner text. Instead, this
* function will return the textContent of the element that contains the banner text. This will
* include the plain text and the text found in the links.
*/

const getBannerText = () =>
screen.getAllByText(/initial credits/).pop().textContent;

const setProfileExtensionEligibility = (isEligible: boolean) => {
profileStore.set({
profile: {
...ProfileStubVariables.PROFILE_STUB,
eligibleForInitialCreditsExtension: isEligible,
},
load,
reload,
updateCache,
});
};

it('should show expiring soon banner to user who created the workspace', async () => {
setupWorkspace(false, false, true, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to see these made explicit, like

    const exhausted = false;
    const expired = false;
    const expiringSoon = true;
    const ownedByMe = true;
    setupWorkspace(exhausted, expired, expiringSoon, ownedByMe);

setProfileExtensionEligibility(true);

component();

await screen.findByText('Workspace credits are expiring soon');
expect(getBannerText()).toMatch(
'Your initial credits are expiring soon. You can request an extension here. For more ' +
'information, read the Using All of Us Initial Credits article on the User Support Hub.'
);
});

it('should show expiring soon banner to user who did not create the workspace', async () => {
setupWorkspace(false, false, true, false);
setProfileExtensionEligibility(true);

component();

await screen.findByText('Workspace credits are expiring soon');
expect(getBannerText()).toMatch(
'This workspace creator’s initial credits are expiring soon. This workspace was ' +
'created by [email protected]. You can request an extension here. For more information, ' +
'read the Using All of Us Initial Credits article on the User Support Hub.'
);
});

it('should show expired banner with option to extend to eligible user who created the workspace', async () => {
setupWorkspace(false, true, false, true);
setProfileExtensionEligibility(true);

component();

await screen.findByText('Workspace credits have expired');
expect(getBannerText()).toMatch(
'Your initial credits have expired. You can request an extension here. For more ' +
'information, read the Using All of Us Initial Credits article on the User Support Hub.'
);
});

it('should show expired banner to user who did not create the workspace and the owner is eligible for extension', async () => {
setupWorkspace(false, true, false, false);
setProfileExtensionEligibility(true);

component();

await screen.findByText('Workspace credits have expired');
expect(getBannerText()).toMatch(
'This workspace creator’s initial credits have expired. This workspace was created by ' +
'[email protected]. You can request an extension here. For more information, read the ' +
'Using All of Us Initial Credits article on the User Support Hub.'
);
});

it('should show expired banner with no option to extend to ineligible user who created the workspace', async () => {
setupWorkspace(false, true, false, true);
setProfileExtensionEligibility(false);

component();

await screen.findByText('This workspace is out of initial credits');
expect(getBannerText()).toMatch(
'Your initial credits have run out. To use the workspace, a valid billing account needs ' +
'to be provided. To learn more about establishing a billing account, read the Paying for Your ' +
'Research article on the User Support Hub.'
);
});

it('should show expired banner to user who did not create the workspace and the owner is not eligible for extension', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to see "exhausted" tests as well

setupWorkspace(false, true, false, false);
setProfileExtensionEligibility(false);

component();

await screen.findByText('This workspace is out of initial credits');
expect(getBannerText()).toMatch(
'This workspace creator’s initial credits have run out. This workspace was created by ' +
'[email protected]. To use the workspace, a valid billing account needs to be provided. ' +
'To learn more about establishing a billing account, read the Paying for Your Research article ' +
'on the User Support Hub.'
);
});
});
Loading