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.

Sign up for GitHub

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

Jump to bottom

Migrate to twenty-ui - input components #7914

Merged
merged 2 commits into from
Oct 28, 2024
Merged

Migrate to twenty-ui - input components #7914

merged 2 commits into from
Oct 28, 2024

Conversation

gitstart-app[bot]
Copy link
Contributor

@gitstart-app gitstart-app bot commented Oct 21, 2024 •
edited
Loading

Description

Migrate Input components:

  • CardPicker
  • Radio
  • RadioGroup
  • Checkbox
  • Toggle
  • IconListViewGrip

Demo

Radio Component on Storybook

Checkbox component on Storybook

Fixes twentyhq/private-issues#92

greptile-apps[bot]
greptile-apps bot reviewed Oct 21, 2024
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR migrates several input components from the twenty-front package to the twenty-ui package, including CardPicker, Radio, RadioGroup, Checkbox, Toggle, and IconListViewGrip.

  • Updated import statements across multiple files to use components from 'twenty-ui' instead of local paths
  • Migrated Checkbox, Radio, Toggle, and other input components to packages/twenty-ui/src/input/components/
  • Added export for 'input' module in packages/twenty-ui/src/index.ts
  • Updated Storybook stories for migrated components in packages/twenty-ui/src/input/components/stories/
  • Removed specific exports for migrated components in packages/twenty-front/tsup.ui.index.tsx

30 file(s) reviewed, 14 comment(s)
Edit PR Review Bot Settings | Greptile

packages/twenty-front/tsup.ui.index.tsx Outdated
export * from './src/modules/ui/input/components/EntityTitleDoubleTextInput';
export * from './src/modules/ui/input/components/IconPicker';
export * from './src/modules/ui/input/components/ImageInput';
export * from './src/modules/ui/input/components/Radio';
export * from './src/modules/ui/input/components/RadioGroup';
export * from './src/modules/ui/input/button/components/Button';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Button component is exported twice. Consider removing this duplicate export

@gitstart-twenty gitstart-twenty changed the title iMigrate to twenty-ui - input components Migrate to twenty-ui - input components Oct 21, 2024
@charlesBochet
Copy link
Member

@gitstart-twenty could you rebase this one?

@charlesBochet charlesBochet self-assigned this Oct 21, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 22, 2024
View reviewed changes
packages/twenty-server/src/main.ts Outdated
@@ -59,6 +63,11 @@
// Create the env-config.js of the front at runtime
generateFrontConfig();

// Enable session - Today it's used only for SSO
if (environmentService.get('AUTH_SSO_ENABLED')) {
app.use(session(getSessionStorageOptions(environmentService)));

Check warning

Code scanning / CodeQL

Clear text transmission of sensitive cookie Medium

Sensitive cookie sent without enforcing SSL encryption.

Copilot Autofix AI 27 days ago

To fix the problem, we need to ensure that the secure attribute is set on cookies used for session management. This can be done by modifying the options passed to the session middleware to include secure: true. This change should be made in the getSessionStorageOptions function to ensure that all session cookies are transmitted securely.

Suggested changeset 1
packages/twenty-server/src/main.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/twenty-server/src/main.ts b/packages/twenty-server/src/main.ts
--- a/packages/twenty-server/src/main.ts
+++ b/packages/twenty-server/src/main.ts
@@ -67,3 +67,6 @@
   if (environmentService.get('AUTH_SSO_ENABLED')) {
-    app.use(session(getSessionStorageOptions(environmentService)));
+    const sessionOptions = getSessionStorageOptions(environmentService);
+    sessionOptions.cookie = sessionOptions.cookie || {};
+    sessionOptions.cookie.secure = true;
+    app.use(session(sessionOptions));
   }
EOF
@@ -67,3 +67,6 @@
if (environmentService.get('AUTH_SSO_ENABLED')) {
app.use(session(getSessionStorageOptions(environmentService)));
const sessionOptions = getSessionStorageOptions(environmentService);
sessionOptions.cookie = sessionOptions.cookie || {};
sessionOptions.cookie.secure = true;
app.use(session(sessionOptions));
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@charlesBochet
Copy link
Member

I think something is wrong with the base

@gitstart-twenty
Copy link
Contributor

I think something is wrong with the base

Thanks, We are investigating this.

@gitstart-twenty
Copy link
Contributor

Hi @charlesBochet, we have resolved PR issues. Please review

@charlesBochet charlesBochet merged commit fc8c9d9 into main Oct 28, 2024
18 checks passed
@charlesBochet charlesBochet deleted the TWNTY-7072 branch October 28, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@charlesBochet charlesBochet charlesBochet approved these changes

Assignees

@charlesBochet charlesBochet

Labels
-PR: awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@charlesBochet @gitstart-twenty