187 tab gestion #194
187 tab gestion #194
Conversation
…ia Link - Add disabled prop to MyLink component - Update styling for disabled links - Replace Inertia Link with MyLink in multiple components - Remove unnecessary import statements - Standardize disabled link appearance across different sections
- Add fallback for missing logo with placeholder text - Implement object-fit for logo images - Add fallback for missing website with disabled button - Update description to show placeholder when empty - Render Gestion component in tabs content
- Implement PieChartComponent for visualizing financial data - Add new chart-related UI components in chart.tsx - Include label logos and financial metrics in Gestion component - Update package.json to remove unnecessary dependencies - Add new label images for Don en Confiance and IDEAS - Enhance data processing and formatting for financial information
|
Caution Review failedThe pull request is closed. WalkthroughMultiple changes have been introduced across the codebase. A new Changes
Sequence Diagram(s)sequenceDiagram
participant EC as "Edit/Show Component"
participant GOD as "getOsblData"
participant PD as "Processed OSBL Data"
participant CC as "Child Components"
EC->>GOD: Call getOsblData(raw OSBL data)
GOD-->>EC: Return processed OSBL data
EC->>CC: Pass processed data for rendering
sequenceDiagram
participant PC as "PieChartComponent"
participant DP as "preparePieData"
participant RC as "Recharts Library"
PC->>DP: Process data prop
DP-->>PC: Return formatted chart data
PC->>RC: Render pie chart with configuration
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 RuboCop (1.69.1)spec/services/contributions/update_service_spec.rb[!] There was an error parsing from /Gemfile:17-------------------------------------------# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-------------------------------------------/usr/lib/ruby/3.1.0/bundler/dsl.rb:357:in spec/requests/contributions_spec.rb[!] There was an error parsing from /Gemfile:17-------------------------------------------# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-------------------------------------------/usr/lib/ruby/3.1.0/bundler/dsl.rb:357:in spec/services/contributions/create_service_spec.rb[!] There was an error parsing from /Gemfile:17-------------------------------------------# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-------------------------------------------/usr/lib/ruby/3.1.0/bundler/dsl.rb:357:in 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
app/models/join_tables/osbls_label.rb (1)
9-9: Consider adding validation for logo_url
A regex-based or built-in URL format validation helps maintain data consistency.app/frontend/lib/formatters.ts (1)
1-14: Consider enhancing the amount formatter with constants and parameterization.The implementation is solid, but could benefit from some improvements:
+const MILLION = 1_000_000 +const CURRENCY = '€' + export function formatAmount (amount: number): string { const formatter = new Intl.NumberFormat('fr-FR', { minimumFractionDigits: 0, maximumFractionDigits: 2 }) - if (amount >= 1000000) { + if (amount >= MILLION) { // For 1M€ or more, show as X,XX M€ - return formatter.format(amount / 1000000) + ' M€' + return formatter.format(amount / MILLION) + ` M${CURRENCY}` } else { // For less than 1M€, show as XX XXX € - return formatter.format(amount) + ' €' + return formatter.format(amount) + ` ${CURRENCY}` } }Would you like me to generate unit tests for this formatting function to ensure correctness across different scenarios?
app/frontend/components/pages/pages/home/FaqSection.tsx (1)
37-49: Consider extracting FAQ links to a configuration object.The repeated MyLink components with similar properties could be generated from a configuration object to improve maintainability.
Example implementation:
+const faqLinks = [ + { href: '/', text: "C'est quoi une association d'intérêt général ?" }, + { href: '/', text: 'Pourquoi donner ?' }, + { href: '/', text: "Qu'est-ce-qu'une réduction d'impôt ?" }, + { href: '/', text: 'À qui donner ?' } +] <ul className='pl-4 sm:pl-8 list-disc flex flex-col space-y-4'> - <li className='underline hover:text-primary'> - <MyLink disabled href='/'> - C'est quoi une association d'intérêt général ? - </MyLink> - </li> - // ... other list items + {faqLinks.map(({ href, text }) => ( + <li key={text} className='underline hover:text-primary'> + <MyLink disabled href={href}> + {text} + </MyLink> + </li> + ))} </ul>app/frontend/lib/osblData.ts (1)
4-62: Consider decomposing the transformation logic.The function is complex with multiple nested transformations. Consider breaking it down into smaller, focused functions for better maintainability.
Example:
function transformFinances(finances: typeof osbl.annual_finances_attributes) { if (!finances) return undefined; return Object.values(finances).map(finance => ({ year: Number(finance.year), budget: numberInput(finance.budget), // ... rest of the transformation })); } function transformLocations(locations: typeof osbl.locations_attributes) { if (!locations) return undefined; return Object.values(locations).map(location => ({ ...location, address_attributes: transformAddress(location.address_attributes) })); } export function getOsblData(osbl: OsblUpdate): NewOsbl { return { ...osbl, creation_year: numberInput(osbl.creation_year), public_utility: Boolean(osbl.public_utility), annual_finances_attributes: transformFinances(osbl.annual_finances_attributes), locations_attributes: transformLocations(osbl.locations_attributes), // ... rest of the transformation }; }app/frontend/components/charts/PieChartComponent.tsx (3)
20-24: Consider using CSS variables for better theme consistency.The color array could be moved to a theme configuration file for better maintainability and consistency across the application.
-const COLORS = ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))', 'hsl(var(--chart-4))', 'hsl(var(--chart-5))'] +// Move to theme.ts +export const CHART_COLORS = ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))', 'hsl(var(--chart-4))', 'hsl(var(--chart-5))']
59-60: Remove console.log statements.Debug logging statements should be removed before production deployment.
- console.log('chartData', chartData) - console.log('chartConfig', chartConfig)
76-86: Consider extracting label calculation logic.The label position calculation logic is complex and could be moved to a separate function for better readability and reusability.
+function calculateLabelPosition(cx: number, cy: number, midAngle: number, innerRadius: number, outerRadius: number) { + const radius = Number(innerRadius) + (Number(outerRadius) - Number(innerRadius)) * 1.20 + const x = Number(cx) + radius * Math.cos(-midAngle * (Math.PI / 180)) + const y = Number(cy) + radius * Math.sin(-midAngle * (Math.PI / 180)) + return { x, y } +} // In the label prop: label={({ cx, cy, midAngle, innerRadius, outerRadius, value }) => { const { x, y } = calculateLabelPosition(cx, cy, midAngle, innerRadius, outerRadius) return ( <text x={x} y={y} dy={8} textAnchor='middle' fill='#333'> {value} % </text> ) }}app/frontend/components/pages/osbl/show/Gestion.tsx (2)
19-29: Extract treasury margin calculation to a utility function.The
getTreasuryMarginfunction contains business logic that could be reused. Consider moving it to a separate utility file.+// In lib/financialCalculations.ts +export function calculateTreasuryMargin(treasury: number, budget: number): number { + return treasury / (budget / 12) +} // In the component: import { calculateTreasuryMargin } from '@/lib/financialCalculations'
47-51: Remove commented code.The commented
watchDragoption should be either removed or documented if it's important for future reference.opts={{ loop: false, slidesToScroll: 'auto', - // watchDrag: false, startIndex }}db/seeds.rb (1)
555-558: Improve Rails.root usage.Use
Rails.root.join().openfor better readability and consistency with Rails best practices.Apply this diff:
- l.logo.attach( - io: File.open(Rails.root.join(label[:logo_path])), - filename: File.basename(label[:logo_path]) - ) + l.logo.attach( + io: Rails.root.join(label[:logo_path]).open, + filename: File.basename(label[:logo_path]) + )🧰 Tools
🪛 GitHub Check: standard_ruby
[failure] 556-556:
Rails/RootPathnameMethods:Rails.rootis aPathname, so you can useRails.root.join(label[:logo_path]).open.🪛 GitHub Actions: CI
[warning] 556-556: Rails/RootPathnameMethods:
Rails.rootis aPathname, so you can useRails.root.join(label[:logo_path]).open.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
app/frontend/assets/inertia.svgis excluded by!**/*.svgapp/frontend/assets/labels/don-en-confiance.pngis excluded by!**/*.pngapp/frontend/assets/labels/label-IDEAS.pngis excluded by!**/*.pngapp/frontend/assets/react.svgis excluded by!**/*.svgapp/frontend/assets/vite_ruby.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
app/frontend/components/charts/PieChartComponent.tsx(1 hunks)app/frontend/components/layout/Header.tsx(1 hunks)app/frontend/components/pages/osbl/show/Gestion.tsx(1 hunks)app/frontend/components/pages/pages/aboutUs/NonProfit.tsx(2 hunks)app/frontend/components/pages/pages/home/FaqSection.tsx(2 hunks)app/frontend/components/pages/pages/joinUs/DonorSection.tsx(3 hunks)app/frontend/components/pages/pages/joinUs/MemberSection.tsx(3 hunks)app/frontend/components/shared/MyCheckbox.tsx(2 hunks)app/frontend/components/shared/MyLink.tsx(1 hunks)app/frontend/components/ui/chart.tsx(1 hunks)app/frontend/lib/formatters.ts(1 hunks)app/frontend/lib/osblData.ts(1 hunks)app/frontend/pages/Contribution/Edit.tsx(1 hunks)app/frontend/pages/Contribution/types.ts(1 hunks)app/frontend/pages/Osbl/Show.tsx(3 hunks)app/models/join_tables/osbls_label.rb(1 hunks)app/models/osbl/annual_finance.rb(1 hunks)app/models/osbl/label.rb(1 hunks)app/serializers/contributions/osbl_data/serializer.rb(2 hunks)db/seeds.rb(2 hunks)lib/file_processor.rb(1 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/frontend/components/ui/chart.tsx
[error] 79-79: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
app/frontend/components/charts/PieChartComponent.tsx
[error] 52-52: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🪛 GitHub Check: standard_ruby
db/seeds.rb
[failure] 551-551:
Lint/UselessAssignment: Useless assignment to variable - osbl_label.
[failure] 556-556:
Rails/RootPathnameMethods: Rails.root is a Pathname, so you can use Rails.root.join(label[:logo_path]).open.
🪛 GitHub Actions: CI
db/seeds.rb
[warning] 556-556: Rails/RootPathnameMethods: Rails.root is a Pathname, so you can use Rails.root.join(label[:logo_path]).open.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (16)
app/frontend/pages/Contribution/Edit.tsx (2)
8-8: Use of external getOsblData import looks good
No issues found with the new import. Just ensure that any old references to the removed function have been replaced throughout the codebase.
23-23: Validate getOsblData transformations
Confirm thatgetOsblDataproperly handles all possibleosblshapes, including edge cases (e.g., missing or incomplete attributes). This helps prevent data mismatches in subsequent form logic.app/frontend/components/ui/chart.tsx (1)
1-77: Solid chart architecture
These components demonstrate a clear structure, with configurable theming and an intuitive context pattern for usage. No major issues identified here.app/models/osbl/label.rb (1)
11-11: LGTM! Reasonable increase in logo size limit.The increase in maximum logo size from 1MB to 5MB is reasonable, especially for high-quality SVG and PNG files, while still maintaining content type validation for security.
app/frontend/components/shared/MyLink.tsx (1)
4-6: LGTM! Well-implemented disabled state for MyLink.The implementation properly handles both the functional and visual aspects of the disabled state, following React best practices.
Also applies to: 8-8, 12-12, 15-22
lib/file_processor.rb (1)
30-36:✅ Verification successful
Verify the need for public access to generate_url.
While the implementation is correct, making this method public suggests it's needed outside the module. Please confirm this is intentional.
Additionally, consider extracting the host/port configuration to avoid potential issues if mailer configuration changes:
+ def self.default_url_options + { + host: Rails.application.config.action_mailer.default_url_options[:host], + port: Rails.application.config.action_mailer.default_url_options[:port] + } + end + def self.generate_url(blob) Rails.application.routes.url_helpers.rails_blob_url( blob, - host: Rails.application.config.action_mailer.default_url_options[:host], - port: Rails.application.config.action_mailer.default_url_options[:port] + **default_url_options ) endLet's verify if this method is used outside the module:
🏁 Script executed:
#!/bin/bash # Search for external usage of FileProcessor.generate_url rg "FileProcessor\.generate_url" --type rubyLength of output: 180
Clarification on Public Accessibility and a Refactor Suggestion
- The grep search confirmed that
FileProcessor.generate_urlis indeed used externally (e.g., inapp/serializers/contributions/osbl_data/serializer.rb), so its public scope appears intentional.- As a maintenance improvement, consider extracting the host/port configuration into a dedicated method to prevent potential issues if mailer settings change in the future.
+ def self.default_url_options + { + host: Rails.application.config.action_mailer.default_url_options[:host], + port: Rails.application.config.action_mailer.default_url_options[:port] + } + end + def self.generate_url(blob) Rails.application.routes.url_helpers.rails_blob_url( blob, - host: Rails.application.config.action_mailer.default_url_options[:host], - port: Rails.application.config.action_mailer.default_url_options[:port] + **default_url_options ) endapp/frontend/components/shared/MyCheckbox.tsx (2)
7-15: LGTM! Props interface changes enhance component flexibility.The changes to make
childrenandonCheckedChangeoptional, along with the addition of theclassNameprop, improve the component's reusability by allowing more flexible usage patterns.
18-38: LGTM! Improved conditional rendering and className handling.The changes improve the component by:
- Properly handling optional
classNamewith nullish coalescing- Adding conditional rendering for the Label component
app/models/osbl/annual_finance.rb (1)
10-11: LGTM! Validation rules now handle edge cases correctly.The changes to allow zero values for
employees_countandbudgetmake sense, as organizations might legitimately have no employees or zero budget in certain situations.app/frontend/components/pages/pages/aboutUs/NonProfit.tsx (1)
53-62:❓ Verification inconclusive
Verify the href change and disabled state.
The link's destination has been changed from '/nous-rejoindre' to '/', and the link is now disabled. Please confirm if this is intentional.
Run this script to check for other occurrences of the original route:
🏁 Script executed:
#!/bin/bash # Search for other components using the '/nous-rejoindre' route rg -l "'/nous-rejoindre'" app/frontend/Length of output: 146
Subject: Confirm Updated Route in NonProfit.tsx vs Other Components
The link in
NonProfit.tsxnow uses'/'and is disabled, but our repository search shows that/nous-rejoindreis still referenced in bothHeader.tsxandTogether.tsx. Please verify whether this change in theNonProfit.tsxcomponent is intentional or if the other components should be updated for consistency.
- NonProfit.tsx (L53-62): Updated route set to
'/'and the link is disabled.- Header.tsx & Together.tsx: Still reference the
/nous-rejoindreroute.app/frontend/components/pages/pages/joinUs/DonorSection.tsx (1)
54-65: LGTM! Clean implementation of the MyLink component.The MyLink component is correctly implemented with proper disabled state and button variants.
app/frontend/components/pages/pages/joinUs/MemberSection.tsx (1)
41-51: LGTM! Clean implementation of the MyLink component.The MyLink component is correctly implemented with proper disabled state and button variants.
app/frontend/lib/osblData.ts (1)
55-55:❓ Verification inconclusive
Verify the impact of setting city to an empty string.
Setting
cityto an empty string might cause validation issues. Consider whether this field is required by the backend.
🏁 Script executed:
#!/bin/bash # Search for city field validations in models rg -A 5 'validates.*:city' --type rubyLength of output: 40
Action: Verify if an empty "city" value is acceptable
The automated search for validations on the "city" attribute did not return any results. Please manually confirm whether the backend requires a non-empty value for "city." If the "city" field should be mandatory, then leaving it as an empty string could lead to issues. Otherwise, if the empty string is acceptable per your design, you may consider adding a comment to clarify this behavior.
app/frontend/components/layout/Header.tsx (1)
115-115: LGTM! Improved disabled state handling.The change to use the
disabledprop instead of CSS classes for disabled state is a better approach, making the code more maintainable and consistent with other components.db/seeds.rb (1)
532-533: LGTM! Added logo paths for labels.The addition of logo paths for the labels enhances the data seeding process by including visual assets.
Also applies to: 545-546
package.json (1)
83-85: LGTM! Added react-is override.The override ensures compatibility with React 19 and prevents version conflicts in the dependency tree.
- Refactor logo attachment method to use more concise syntax - Remove unnecessary variable assignment in label creation - Optimize file opening method for logo attachment
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
db/seeds.rb(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (2)
db/seeds.rb (2)
562-564: LGTM!The conditional loading of development seeds is a good practice.
522-534:✅ Verification successful
Verify the logo file paths and ensure proper error handling.
The logo paths are hardcoded and assume the existence of specific files in the frontend assets directory. This could lead to runtime errors if the files are missing.
Run this script to verify the existence of the logo files:
🏁 Script executed:
#!/bin/bash # Description: Check if the logo files exist in the specified paths for logo in "don-en-confiance.png" "label-IDEAS.png"; do fd --type f "$logo" app/frontend/assets/labels/ doneLength of output: 321
Logo Paths Verified – No Changes Required
The verification script confirms that the logo files (both "don-en-confiance.png" and "label-IDEAS.png") exist in the expected directory. The seed file’s update with the new
logo_pathattribute is therefore valid. Please ensure that your Active Storage attachment logic also includes robust error handling for potential future cases where a file might be missing.
- Verified file path for "don-en-confiance.png":
app/frontend/assets/labels/don-en-confiance.png- Verified file path for "label-IDEAS.png":
app/frontend/assets/labels/label-IDEAS.png
- Update contributions controller to redirect to contribution show page after create/update - Modify OsblData serializer to handle different serialization modes - Improve PieChartComponent data processing using Object.fromEntries - Fix minor typo in OsblFinanceSheet label - Update NewOsbl type to make logo_url optional in labels
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/serializers/contributions/osbl_data/serializer.rb (1)
43-57:⚠️ Potential issueOptimize database queries and add error handling.
The current implementation has several issues:
- N+1 query issue when finding labels
- No error handling for invalid label_id
- Duplicated code between Array and Hash cases
- No validation of blob presence
Apply this diff to address the issues:
def transform_labels! return if @params["osbls_labels_attributes"].blank? + # Convert Hash to Array if needed + labels_attributes = @params["osbls_labels_attributes"] + labels_attributes = labels_attributes.values if labels_attributes.is_a?(Hash) + + # Fetch all labels in one query + label_ids = labels_attributes.map { |attrs| attrs["label_id"] } + labels = Osbl::Label.where(id: label_ids).index_by(&:id) + + labels_attributes.each do |label_attrs| + label = labels[label_attrs["label_id"]] + next unless label # Skip if label not found + + # Generate URL only if blob exists + if label.logo&.blob + label_attrs["logo_url"] = FileProcessor.generate_url(label.logo.blob) + end + end
🧹 Nitpick comments (5)
app/frontend/components/charts/PieChartComponent.tsx (5)
22-40: Enhance type safety and internationalization.Consider these improvements:
- Add type safety for the 'Information manquante' fallback.
- Consider extracting the fallback text for localization.
+const FALLBACK_TEXT = 'Information manquante' // TODO: Add to translations + +type PieDataItem = { + name: string + percent: number + amount?: number + fill: string +} + -function preparePieData (data: FundRecord[] | undefined): Array<{ name: string, percent: number, amount?: number, fill: string }> { +function preparePieData (data: FundRecord[] | undefined): PieDataItem[] { if (data === undefined) { return [{ - name: 'Information manquante', + name: FALLBACK_TEXT, percent: 100, fill: 'hsl(var(--muted))' }] } return data.map((item, index) => ({ name: item.type, percent: item.percent, amount: item.amount, fill: getRandomColor(index) })) }
42-45: Add JSDoc documentation for the props interface.Consider adding documentation to improve code maintainability.
+/** + * Props for the PieChartComponent + * @property {FundRecord[] | undefined} data - The data to display in the pie chart + * @property {string} title - The title to display above the chart + */ interface PieChartComponentProps { data: FundRecord[] | undefined title: string }
60-61: Remove debug console.log statements.Remove these debug statements before merging to production.
- console.log('chartData', chartData) - console.log('chartConfig', chartConfig)
77-87: Extract label positioning calculation for better maintainability.The label positioning logic is complex and could benefit from being extracted into a separate function.
+const calculateLabelPosition = ( + cx: number, + cy: number, + midAngle: number, + innerRadius: number, + outerRadius: number +) => { + const radius = innerRadius + (outerRadius - innerRadius) * 1.20 + const x = cx + radius * Math.cos(-midAngle * (Math.PI / 180)) + const y = cy + radius * Math.sin(-midAngle * (Math.PI / 180)) + return { x, y } +} + label={({ cx, cy, midAngle, innerRadius, outerRadius, value }) => { - const radius = Number(innerRadius) + (Number(outerRadius) - Number(innerRadius)) * 1.20 - const x = Number(cx) + radius * Math.cos(-midAngle * (Math.PI / 180)) - const y = Number(cy) + radius * Math.sin(-midAngle * (Math.PI / 180)) + const { x, y } = calculateLabelPosition( + Number(cx), + Number(cy), + Number(midAngle), + Number(innerRadius), + Number(outerRadius) + ) return ( <text x={x} y={y} dy={8} textAnchor='middle' fill='#333'> {value} % </text> ) }}
97-117: Optimize tooltip content with memoization.The tooltip content formatter could be memoized to prevent unnecessary re-renders.
+import { useMemo } from 'react' + +const TooltipContent = ({ name, value, item }: { name: string, value: number, item: any }) => ( + <div> + <p className='font-semibold mb-1'>{name}</p> + <div className='flex justify-between gap-8'> + <p>Pourcentage :</p> + <p>{value} %</p> + </div> + <div className='flex justify-between gap-8'> + <p>Montant :</p> + <p> + {item.payload.amount !== undefined + ? formatAmount(item.payload.amount) + : <span className='text-muted-foreground'>-</span>} + </p> + </div> + </div> +) + content={ <ChartTooltipContent - formatter={(value, name, item) => { - return ( - <div> - <p className='font-semibold mb-1'>{name}</p> - <div className='flex justify-between gap-8'> - <p>Pourcentage :</p> - <p>{value} %</p> - </div> - <div className='flex justify-between gap-8'> - <p>Montant :</p> - <p> - {item.payload.amount !== undefined - ? formatAmount(item.payload.amount) - : <span className='text-muted-foreground'>-</span>} - </p> - </div> - </div> - ) - }} + formatter={(value, name, item) => ( + <TooltipContent name={name} value={value} item={item} /> + )} /> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/controllers/users/contributions_controller.rb(3 hunks)app/frontend/components/charts/PieChartComponent.tsx(1 hunks)app/frontend/components/pages/contribution/new/OsblFinances/OsblFinanceSheet.tsx(1 hunks)app/frontend/pages/Contribution/types.ts(1 hunks)app/serializers/contributions/osbl_data/serializer.rb(2 hunks)app/services/contributions/create_service.rb(1 hunks)app/services/contributions/update_service.rb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/frontend/components/pages/contribution/new/OsblFinances/OsblFinanceSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/frontend/pages/Contribution/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (6)
app/services/contributions/create_service.rb (1)
19-19: LGTM! Improved return value.Returning the contribution object provides more contextual information and maintains consistency with the update service.
app/services/contributions/update_service.rb (1)
24-24: LGTM! Improved return value.Returning the contribution object provides more contextual information and maintains consistency with the create service.
app/serializers/contributions/osbl_data/serializer.rb (1)
4-7: LGTM! Added mode parameter.The mode parameter effectively controls when to transform labels, making the serializer more flexible.
app/controllers/users/contributions_controller.rb (2)
25-25: LGTM! Added display mode.The :display mode ensures labels are transformed only when showing the contribution, improving performance.
45-45: LGTM! Improved redirects.Redirecting to the specific contribution after create/update provides better user experience by showing the result of their action.
Also applies to: 70-70
app/frontend/components/charts/PieChartComponent.tsx (1)
1-20: LGTM! Well-organized imports and theme-aware color constants.The imports are well-structured, and using CSS variables for colors enables consistent theming across the application.
- Modify redirect expectations in contributions request specs - Simplify service spec assertions by removing explicit message checks - Adjust test cases to match updated service return values
Summary by CodeRabbit
New Features
UI Enhancements
Dependency Management