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

Admin ability to soft delete and update phone of a member #719

Merged
merged 7 commits into from
Oct 16, 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
1 change: 1 addition & 0 deletions adminapp/src/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export default {
getMembers: (data) => get(`/adminapi/v1/members`, data),
getMember: ({ id, ...data }) => get(`/adminapi/v1/members/${id}`, data),
updateMember: ({ id, ...data }) => post(`/adminapi/v1/members/${id}`, data),
softDeleteMember: ({ id, ...data }) => post(`/adminapi/v1/members/${id}/close`, data),
changeMemberEligibility: ({ id, ...data }) =>
post(`/adminapi/v1/members/${id}/eligibilities`, data),

Expand Down
91 changes: 87 additions & 4 deletions adminapp/src/pages/MemberDetailPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,24 @@ import { useUser } from "../hooks/user";
import { dayjs } from "../modules/dayConfig";
import Money from "../shared/react/Money";
import SafeExternalLink from "../shared/react/SafeExternalLink";
import { Divider, Typography, Switch, Button, Chip } from "@mui/material";
import useToggle from "../shared/react/useToggle";
import DeleteIcon from "@mui/icons-material/Delete";
import {
Divider,
Typography,
Switch,
Button,
Chip,
DialogTitle,
Dialog,
DialogContent,
DialogActions,
} from "@mui/material";
import IconButton from "@mui/material/IconButton";
import { makeStyles } from "@mui/styles";
import isEmpty from "lodash/isEmpty";
import React from "react";
import { formatPhoneNumberIntl } from "react-phone-number-input";
import { formatPhoneNumber, formatPhoneNumberIntl } from "react-phone-number-input";
import { useParams } from "react-router-dom";

export default function MemberDetailPage() {
Expand Down Expand Up @@ -96,8 +109,15 @@ export default function MemberDetailPage() {
{ label: "Created At", value: dayjs(model.createdAt) },
{
label: "Deleted At",
value: model.softDeletedAt && dayjs(model.softDeletedAt),
hideEmpty: true,
value: (
<InlineSoftDelete
id={model.id}
name={model.name}
phone={formatPhoneNumber("+" + model.phone)}
softDeletedAt={model.softDeletedAt}
onSoftDelete={(member) => setModel(member)}
/>
),
},
]}
/>
Expand Down Expand Up @@ -405,6 +425,69 @@ function ImpersonateButton({ id }) {
);
}

function InlineSoftDelete({ id, name, phone, softDeletedAt, onSoftDelete }) {
const { enqueueErrorSnackbar } = useErrorSnackbar();
const showModal = useToggle(false);
const handleDelete = () => {
api
.softDeleteMember({ id: id })
.then((r) => {
onSoftDelete(r.data);
showModal.turnOff();
})
.catch(enqueueErrorSnackbar);
};
const { canWrite } = useRoleAccess();
let display = "-";
if (canWrite("admin_members")) {
display = (
<>
{"- "}
<IconButton onClick={() => showModal.turnOn()}>
<DeleteIcon color="error" />
</IconButton>
</>
);
}
return (
<>
{softDeletedAt ? dayjs(softDeletedAt).format("lll") : display}
<Dialog open={showModal.isOn} onClose={showModal.turnOff}>
<DialogTitle>Confirm Soft Deletion</DialogTitle>
<DialogContent>
<Typography sx={{ mt: 1 }} color="error">
Member deletion CANNOT be undone. Ensure that you know what you are doing
before continuing.
</Typography>
<Typography sx={{ mt: 1 }}>
"Soft delete" means an account is closed indefinitely but keeps some important
records attached to the account in case we need to refer to past data. It
replaces the member's phone and email with unreachable values, preventing any
further account access.
</Typography>
</DialogContent>
<DialogActions>
<Button onClick={handleDelete} variant="contained" color="error" size="small">
Soft Delete
<br />
{name}
<br />
{phone}
</Button>
<Button
variant="outlined"
color="secondary"
size="small"
onClick={showModal.turnOff}
>
Cancel
</Button>
</DialogActions>
</Dialog>
</>
);
}

const useStyles = makeStyles((theme) => ({
impersonate: {
marginLeft: theme.spacing(2),
Expand Down
13 changes: 12 additions & 1 deletion adminapp/src/pages/MemberForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function MemberForm({
}) {
return (
<FormLayout
title={"Update Member"}
title="Update Member"
subtitle="Edit member account information"
onSubmit={onSubmit}
isBusy={isBusy}
Expand Down Expand Up @@ -64,6 +64,17 @@ export default function MemberForm({
onChange={setFieldFromInput}
/>
</ResponsiveStack>
<TextField
{...register("phone")}
label="Phone"
name="phone"
value={resource.phone || ""}
type="tel"
variant="outlined"
fullWidth
helperText="11-digit US phone number, begins with 1."
Copy link
Member

Choose a reason for hiding this comment

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

What happens if it's taken? We get a nice error?

Copy link
Member

Choose a reason for hiding this comment

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

Mostly I want to make sure we explain that the member with the number already, would need to be deleted first.

onChange={setFieldFromInput}
/>
<Roles roles={resource.roles} setRoles={(r) => setField("roles", r)} />
<Divider />
<FormLabel>Legal Entity</FormLabel>
Expand Down
25 changes: 24 additions & 1 deletion adminapp/src/shared/react/useAsyncFetch.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import React from "react";
* The state is cleared as soon as it is fetched, since it can get stale quickly
* as it does not behave like React state (ie it persists between refreshses).
* @param {Location=} options.location Must be provided for pullFromState to be used.
* @param {boolean=} options.cache If true, cache the API response using the options as a key.
* When caching, the arguments passed to makeRequest MUST be serializable using JSON.stringify.
* @returns {{state, asyncFetch, error, loading}}
*/
const useAsyncFetch = (makeRequest, options) => {
Expand All @@ -23,6 +25,7 @@ const useAsyncFetch = (makeRequest, options) => {
location,
pickData,
pullFromState,
cache,
} = options || {};
if (pullFromState && get(location, ["state", pullFromState])) {
defaultVal = location.state[pullFromState];
Expand All @@ -32,22 +35,42 @@ const useAsyncFetch = (makeRequest, options) => {
const [state, setState] = React.useState(defaultVal);
const [error, setError] = React.useState(null);
const [loading, setLoading] = React.useState(!doNotFetchOnInit);
const cacheRef = React.useRef({});

const asyncFetch = React.useCallback(
(...args) => {
setLoading(true);
setError(false);
let cacheKey;
if (cache) {
// If we're caching, and the entry is in the cache,
// then return it from the cache. Use a 200ms delay to mimic a very fast
// network call, since when callers use this they normally expect some delay.
// We can add options to control this in the future.
cacheKey = "" + makeRequest + JSON.stringify(args);
if (cacheRef.current[cacheKey]) {
return Promise.delay(200).then(() => {
const st = cacheRef.current[cacheKey];
setState(st);
setLoading(false);
return st;
});
}
}
return makeRequest(...args)
.then((x) => {
const st = pickData ? x.data : x;
setState(st);
if (cache) {
cacheRef.current[cacheKey] = st;
}
return st;
})
.tapCatch((e) => setError(e))
.tap(() => setLoading(false))
.tapCatch(() => setLoading(false));
},
[makeRequest, pickData]
[cache, makeRequest, pickData]
);

React.useEffect(() => {
Expand Down
11 changes: 11 additions & 0 deletions adminapp/src/shared/react/useUnmountEffect.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from "react";

/**
* Just a `React.useEffect(() => cb, [])` that is more declarative than
* doing it in line and disabling eslint.
* @param {function} cb
*/
export default function useUnmountEffect(cb) {
// eslint-disable-next-line react-hooks/exhaustive-deps
React.useEffect(() => cb, []);
}
15 changes: 15 additions & 0 deletions adminapp/src/shared/setRef.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import has from "lodash/has";

/**
* Call ref(v) or ref.current = v;.
*/
export default function setRef(ref, value) {
if (!ref) {
return;
}
if (has(ref, "current")) {
ref.current = value;
return;
}
ref(value);
}
6 changes: 5 additions & 1 deletion lib/suma/member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,11 @@ def after_save
def validate
super
self.validates_presence(:phone)
self.validates_unique(:phone)
self.validates_unique(
:phone,
message: "is already taken. If you're trying to duplicate a member, " \
"make sure that you soft-delete their account first.",
)
unless self.soft_deleted?
self.validates_format(Suma::PhoneNumber::US::REGEXP, :phone, message: "is not an 11 digit US phone number")
end
Expand Down
15 changes: 13 additions & 2 deletions spec/suma/admin_api/members_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,22 @@ def make_item(i)
it "updates the member" do
member = Suma::Fixtures.member.create

post "/v1/members/#{member.id}", name: "b 2", email: "[email protected]"
post "/v1/members/#{member.id}", name: "b 2", email: "[email protected]", phone: "12223334444"

expect(last_response).to have_status(200)
expect(last_response).to have_json_body.
that_includes(id: member.id, name: "b 2", email: "[email protected]")
that_includes(id: member.id, name: "b 2", email: "[email protected]", phone: "12223334444")
end

it "400s if a phone number is taken, with instructions message for admins" do
member = Suma::Fixtures.member.create
member_taken = Suma::Fixtures.member.create(phone: "15553335555")

post "/v1/members/#{member.id}", phone: member_taken.phone

expect(last_response).to have_status(400)
message = /duplicate a member, make sure that you soft-delete their account first/
expect(last_response).to have_json_body.that_includes(error: include(message:))
end

it "replaces roles if given" do
Expand Down
Loading