-
Notifications
You must be signed in to change notification settings - Fork 3
Hide Notification, hide files upload & mail config by organization type, app token #334
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: main
Are you sure you want to change the base?
Changes from 3 commits
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 |
|---|---|---|
|
|
@@ -9,6 +9,8 @@ import { SettingsExperiencePage } from '~/pages/settings/account/ExperiencePage' | |
| import { getPluginsSettingsRoutes } from '@/app/hooks/usePluginsRouter'; | ||
| import { Skeleton } from 'erxes-ui'; | ||
| import { SettingsPageEffect } from '@/settings/components/SettingsPageEffect'; | ||
| import { currentOrganizationState } from 'ui-modules'; | ||
| import { useAtomValue } from 'jotai'; | ||
|
|
||
| const SettingsProfile = lazy(() => | ||
| import('~/pages/settings/account/ProfilePage').then((module) => ({ | ||
|
|
@@ -104,6 +106,8 @@ const PropertiesSettins = lazy(() => | |
| ); | ||
|
|
||
| export function SettingsRoutes() { | ||
| const currentOrganization = useAtomValue(currentOrganizationState); | ||
| const isOs = currentOrganization?.type === 'os'; | ||
| return ( | ||
| <Suspense fallback={<Skeleton />}> | ||
| <Routes> | ||
|
|
@@ -112,10 +116,10 @@ export function SettingsRoutes() { | |
| element={<Navigate to={`${SettingsPath.Profile}`} replace />} | ||
| /> | ||
| <Route path={SettingsPath.Profile} element={<SettingsProfile />} /> | ||
| <Route | ||
| {/* <Route | ||
|
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. Remove stale commented-out routes (e.g. Notification, Experience, General) if they are no longer needed to keep the code clean. |
||
| path={SettingsPath.Notification} | ||
| element={<NotificationSettingsRoutes />} | ||
| /> | ||
| /> */} | ||
| <Route | ||
| path={SettingsPath.ChangePassword} | ||
| element={<SettingsChangePassword />} | ||
|
|
@@ -124,14 +128,18 @@ export function SettingsRoutes() { | |
| path={SettingsPath.Experience} | ||
| element={<SettingsExperiencePage />} | ||
| /> */} | ||
| <Route | ||
| path={SettingsWorkspacePath.FileUpload} | ||
| element={<SettingsFileUpload />} | ||
| /> | ||
| <Route | ||
| path={SettingsWorkspacePath.MailConfig} | ||
| element={<SettingsMailConfig />} | ||
| /> | ||
| {isOs && ( | ||
| <Route | ||
| path={SettingsWorkspacePath.FileUpload} | ||
| element={<SettingsFileUpload />} | ||
| /> | ||
| )} | ||
| {isOs && ( | ||
| <Route | ||
| path={SettingsWorkspacePath.MailConfig} | ||
| element={<SettingsMailConfig />} | ||
| /> | ||
| )} | ||
| {/* <Route | ||
| path={SettingsWorkspacePath.General} | ||
| element={<GeneralSettings />} | ||
|
|
@@ -161,7 +169,10 @@ export function SettingsRoutes() { | |
| path={SettingsWorkspacePath.AutomationsCatchAll} | ||
| element={<AutomationSettingsRoutes />} | ||
| /> | ||
| <Route path={SettingsWorkspacePath.Apps} element={<AppsSettings />} /> | ||
| <Route | ||
| path={SettingsWorkspacePath.AppsCatchAll} | ||
| element={<AppsSettings />} | ||
| /> | ||
| <Route | ||
| path={SettingsWorkspacePath.Properties} | ||
| element={<PropertiesSettins />} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,26 @@ | ||
| import { AppsHeader } from '@/settings/apps/components/AppsHeader'; | ||
| import { appsSettingsColumns } from '@/settings/apps/components/table/AppsSettingsColumns'; | ||
| import { useAppsTokens } from '@/settings/apps/hooks/useAppsTokens'; | ||
| import { IApp } from '@/settings/apps/types'; | ||
| import { RecordTable } from 'erxes-ui'; | ||
|
|
||
| export const AppsSettings = () => { | ||
| const { apps, loading } = useAppsTokens(); | ||
| return ( | ||
| <> | ||
| <AppsHeader /> | ||
| </> | ||
| <section className="max-w-xl w-full mx-auto"> | ||
| <legend className="font-semibold text-lg pt-4 pb-6">Apps settings</legend> | ||
|
|
||
| <RecordTable.Provider | ||
| columns={appsSettingsColumns} | ||
| data={apps as IApp[]} | ||
| > | ||
| <RecordTable className='w-full'> | ||
| <RecordTable.Header /> | ||
| <RecordTable.Body> | ||
| <RecordTable.RowList /> | ||
| {loading && <RecordTable.RowSkeleton rows={30} />} | ||
| </RecordTable.Body> | ||
| </RecordTable> | ||
| </RecordTable.Provider> | ||
| </section> | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,159 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useAddAppToken } from '@/settings/apps/hooks/useAddAppToken'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCreateAppForm } from '@/settings/apps/hooks/useCreateAppForm'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { TCreateAppForm } from '@/settings/apps/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { SettingsWorkspacePath } from '@/types/paths/SettingsPath'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IconChevronLeft, IconPlus } from '@tabler/icons-react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Button, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Checkbox, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Checkbox, |
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
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.
🛠️ Refactor suggestion
Normalize payload before mutation (trim, gate fields, ISO date)
Prevents sending empty userGroupId or expireDate when toggles disable them; avoids backend reject.
Apply:
const onSubmit = (data: TCreateAppForm) => {
- appsAdd({
- variables: data,
+ const variables = {
+ ...data,
+ name: data.name.trim(),
+ userGroupId: data.allowAllPermission
+ ? undefined
+ : data.userGroupId?.trim() || undefined,
+ expireDate: data.noExpire
+ ? undefined
+ : data.expireDate?.toISOString(),
+ };
+ appsAdd({
+ variables,
onCompleted: () => {
toast({ title: 'Created a token' });
navigate(SettingsWorkspacePath.Apps);
reset();
},
onError: (error) =>
toast({ title: error.message, variant: 'destructive' }),
});
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onSubmit = (data: TCreateAppForm) => { | |
| appsAdd({ | |
| variables: data, | |
| onCompleted: () => { | |
| toast({ title: 'Created a token' }); | |
| navigate(SettingsWorkspacePath.Apps); | |
| reset(); | |
| }, | |
| onError: (error) => | |
| toast({ title: error.message, variant: 'destructive' }), | |
| }); | |
| }; | |
| const onSubmit = (data: TCreateAppForm) => { | |
| const variables = { | |
| ...data, | |
| name: data.name.trim(), | |
| userGroupId: data.allowAllPermission | |
| ? undefined | |
| : data.userGroupId?.trim() || undefined, | |
| expireDate: data.noExpire | |
| ? undefined | |
| : data.expireDate?.toISOString(), | |
| }; | |
| appsAdd({ | |
| variables, | |
| onCompleted: () => { | |
| toast({ title: 'Created a token' }); | |
| navigate(SettingsWorkspacePath.Apps); | |
| reset(); | |
| }, | |
| onError: (error) => | |
| toast({ title: error.message, variant: 'destructive' }), | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx around
lines 30 to 41, the mutation is sending raw form data which can include empty or
disabled fields and non-ISO dates; normalize the payload before calling appsAdd
by trimming string fields (e.g., name, description), omitting userGroupId when
the corresponding toggle is off or the value is empty, omitting expireDate when
the expiration toggle is off, and converting any expireDate to an ISO string
(e.g., expireDate.toISOString()) before passing variables; then call appsAdd
with this sanitized payload so the backend doesn't receive empty/invalid fields.
Outdated
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.
suggestion: Expire date field default value may be confusing.
Defaulting expireDate to the current date could cause confusion. Suggest prompting the user to choose an expiration date or setting a more meaningful default.
Suggested implementation:
const { control, handleSubmit, formState } = useForm({
defaultValues: {
name: "",
expireDate: undefined, // No default, user must choose
// ...other defaults
},
}); {!noExpire && (
<Form.Field
control={control}
name="expireDate"
render={({ field }) => (
<Form.Item className="col-span-2">
<Form.Label>Expiration Date</Form.Label>
<Form.Control>
<Input
{...field}
type="date"
placeholder="Select expiration date"
min={new Date().toISOString().split("T")[0]} // Prevent past dates
/>
</Form.Control>
<Form.Message />
</Form.Item>
)}
/>If you have validation logic elsewhere that expects expireDate to always be set, you may need to update it to handle the case where the field is empty or undefined. Also, ensure that the UI clearly indicates that the user must select an expiration date.
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.
fix
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.
Thanks for the quick update!
Currently, the default value for expireDate in your form hook is still set to new Date(), which means the expiration date will default to today unless the user changes it. Your schema validation does require the user to set an expiration date when noExpire is false, and the UI uses a DatePicker that defaults to today if no value is set.
If you want to ensure users actively choose an expiration date (rather than accepting today's date by default), you could set the default to undefined and rely on your validation and UI to prompt the user. Otherwise, your current setup is functional and will prevent empty expiration dates when required.
Let me know if you'd like further suggestions or a more detailed review of the updated logic!
Uh oh!
There was an error while loading. Please reload this page.