-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add popups for import and add users on sponsors global #963
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
1c5b836
338a22d
b3c4f7a
fc08562
cbc2f4b
20ea60d
19733a7
a9ef1a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| import React, { useEffect, useState } from "react"; | ||
| import T from "i18n-react/dist/i18n-react"; | ||
| import { connect } from "react-redux"; | ||
| import { | ||
| Box, | ||
| Button, | ||
| Card, | ||
| Dialog, | ||
| DialogActions, | ||
| DialogContent, | ||
| DialogTitle, | ||
| Divider, | ||
| IconButton, | ||
| Typography | ||
| } from "@mui/material"; | ||
| import CloseIcon from "@mui/icons-material/Close"; | ||
| import { FormikProvider, useFormik } from "formik"; | ||
| import * as yup from "yup"; | ||
| import CheckBoxList from "openstack-uicore-foundation/lib/components/mui/checkbox-list"; | ||
| import { | ||
| fetchSponsorUsersBySummit, | ||
| importSponsorUsers | ||
| } from "../../../../actions/sponsor-users-actions"; | ||
| import SummitsDropdown from "../../../../components/mui/summits-dropdown"; | ||
| import MuiFormikAsyncAutocomplete from "../../../../components/mui/formik-inputs/mui-formik-async-select"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomrndom This popup (and the legacy ones touched in this PR) import several components from local
This is the documented pattern: https://github.com/fntechgit/ftn-docsnsklz/blob/main/patterns/show-admin/summit-admin-reuse-before-build.md - check uicore for an existing equivalent before adding/using a local one. Two of these aren't just import-path noise: None of these 5 local files were introduced by this PR - just flagging since this PR already imports |
||
| import { querySponsors } from "../../../../actions/sponsor-actions"; | ||
|
|
||
| const SponsorGlobalImportUsersPopup = ({ | ||
| currentSummit, | ||
| onClose, | ||
| importSponsorUsers | ||
| }) => { | ||
| const [selectedSummit, setSelectedSummit] = useState(null); | ||
| const [userOptions, setUserOptions] = useState(null); | ||
| const [selectedUsers, setSelectedUsers] = useState([]); | ||
|
|
||
| const formik = useFormik({ | ||
| initialValues: { sponsor: { id: "", name: "" } }, | ||
| validationSchema: yup.object({ | ||
| sponsor: yup.object({ | ||
| id: yup.string().required(T.translate("validation.required")) | ||
| }) | ||
| }), | ||
|
Comment on lines
+44
to
+49
|
||
| onSubmit: () => {}, | ||
| validateOnBlur: false, | ||
| enableReinitialize: true | ||
| }); | ||
|
|
||
| const sponsorId = formik.values.sponsor?.value; | ||
| const companyId = formik.values.sponsor?.companyId; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
smarcet marked this conversation as resolved.
|
||
| useEffect(() => { | ||
| if (selectedSummit && sponsorId && companyId) { | ||
| fetchSponsorUsersBySummit( | ||
| currentSummit.id, | ||
| selectedSummit, | ||
| companyId, | ||
| 1 | ||
| ).then((userData) => { | ||
|
tomrndom marked this conversation as resolved.
|
||
| setUserOptions(userData); | ||
| setSelectedUsers([]); | ||
| }); | ||
| } | ||
| }, [selectedSummit, sponsorId, companyId]); | ||
|
tomrndom marked this conversation as resolved.
smarcet marked this conversation as resolved.
smarcet marked this conversation as resolved.
|
||
|
|
||
| const handleLoadMoreUsers = () => { | ||
|
tomrndom marked this conversation as resolved.
|
||
| if (userOptions.current_page < userOptions.last_page) { | ||
| fetchSponsorUsersBySummit( | ||
| currentSummit.id, | ||
| selectedSummit, | ||
| companyId, | ||
| userOptions.current_page + 1 | ||
| ).then((userData) => { | ||
| setUserOptions((value) => ({ | ||
| ...userData, | ||
| data: [...value.data, ...userData.data] | ||
| })); | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| const handleClose = () => { | ||
| onClose(); | ||
|
tomrndom marked this conversation as resolved.
|
||
| }; | ||
|
|
||
| const handleImport = async () => { | ||
| importSponsorUsers( | ||
| sponsorId, | ||
| companyId, | ||
| selectedSummit, | ||
| selectedUsers | ||
| ).then(() => { | ||
| onClose(); | ||
| }); | ||
| }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| const handleSelectOnChange = (items, all = false) => { | ||
| if (all) { | ||
| setSelectedUsers("all"); | ||
| } else { | ||
| setSelectedUsers(items); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <Dialog open onClose={handleClose} maxWidth="sm" fullWidth> | ||
| <DialogTitle | ||
| sx={{ display: "flex", justifyContent: "space-between", p: 2 }} | ||
| component="div" | ||
| > | ||
| <Typography variant="h5"> | ||
| {T.translate("sponsor_users.import_users.title")} | ||
| </Typography> | ||
| <IconButton size="large" sx={{ p: 0 }} onClick={handleClose}> | ||
| <CloseIcon fontSize="large" /> | ||
| </IconButton> | ||
| </DialogTitle> | ||
| <Divider /> | ||
| <FormikProvider value={formik}> | ||
| <DialogContent sx={{ p: 2 }}> | ||
| <SummitsDropdown | ||
| onChange={(val) => { | ||
| setSelectedSummit(val); | ||
| setUserOptions(null); | ||
| setSelectedUsers([]); | ||
| }} | ||
|
Comment on lines
+180
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomrndom Confirmed — There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomrndom Following up on this thread - the suggested fix was applied ( Root cause: Verified this empirically (not just by reading the code): rendered const queryFunction = jest.fn((input, param, callback) => {
callback([{ id: 1, name: `sponsor-for-${param}` }]);
return Promise.resolve();
});
// 1st render: queryParams=["summitA"] -> queryFunction called once
// rerender with queryParams=["summitB"], no search term typed
// queryFunction call count stayed at 1 -> confirmed no refetch happenedSuggested fix: key the Autocomplete on
Comment on lines
+180
to
+185
|
||
| excludeSummitIds={[currentSummit.id]} | ||
| /> | ||
| {selectedSummit && ( | ||
| <Box sx={{ mb: 2, mt: 2 }}> | ||
| <MuiFormikAsyncAutocomplete | ||
| name="sponsor" | ||
| queryFunction={querySponsors} | ||
| queryParams={[selectedSummit]} | ||
| placeholder={T.translate( | ||
| "sponsor_users.process_request.select_sponsor" | ||
| )} | ||
| formatOption={(item) => ({ | ||
| value: item.id, | ||
| label: item.name, | ||
| companyId: item.companyId | ||
| })} | ||
| formatSelectedValue={(s) => ({ | ||
| id: parseInt(s.value), | ||
| name: s.label, | ||
| companyId: s.companyId | ||
| })} | ||
| /> | ||
| </Box> | ||
| )} | ||
| {selectedSummit && sponsorId && userOptions && ( | ||
| <> | ||
| <Typography | ||
| variant="body1" | ||
| gutterBottom | ||
| sx={{ color: "text.secondary", mt: 2 }} | ||
| > | ||
| {T.translate("sponsor_users.import_users.select_users")} | ||
| </Typography> | ||
| <Card variant="outlined"> | ||
| <CheckBoxList | ||
| items={userOptions.data.map((it) => ({ | ||
| id: it.id, | ||
| name: | ||
| it.first_name && it.last_name | ||
| ? `${it.first_name} ${it.last_name}` | ||
| : it.email | ||
| }))} | ||
| onChange={handleSelectOnChange} | ||
| label={T.translate( | ||
| "sponsor_users.import_users.select_all_users" | ||
| )} | ||
| loadMoreData={handleLoadMoreUsers} | ||
| boxHeight="200px" | ||
| /> | ||
| </Card> | ||
| </> | ||
| )} | ||
| </DialogContent> | ||
| </FormikProvider> | ||
| <Divider sx={{ margin: "10px 0px 20px 0px" }} /> | ||
| <DialogActions> | ||
| <Button | ||
| fullWidth | ||
| variant="contained" | ||
| onClick={handleImport} | ||
| disabled={selectedUsers.length === 0} | ||
| > | ||
| {T.translate("sponsor_users.import_users.import_users")} | ||
| </Button> | ||
| </DialogActions> | ||
| </Dialog> | ||
| ); | ||
| }; | ||
|
|
||
| const mapStateToProps = ({ currentSummitState }) => ({ | ||
| currentSummit: currentSummitState.currentSummit | ||
| }); | ||
|
|
||
| export default connect(mapStateToProps, { | ||
| importSponsorUsers | ||
| })(SponsorGlobalImportUsersPopup); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomrndom Confirmed. I traced
postRequest's internal handler (oein the minifiedopenstack-uicore-foundationbundle) — on error it dispatches the error handler and callsreject(...), so removing the.catch(console.log)here does make the returned promise reject on failure where it previously always resolved.new-user-popup.js:38(sendSponsorUserInvite(values.email).finally(...)) has no.catch(), so a failed invite from that (untouched) screen will now surface as an unhandled promise rejection. Needs a.catch()added there (or a rethrow-and-log wrapper kept in the action) as part of this PR, since it's the one changing the contract.