added support for column name localization #56
Conversation
There was a problem hiding this comment.
@Mitroz I am so sorry for the delayed approval of this change, we will get a new release coming soon with this included!
mitchelsellers
merged commit 921561e
into
IowaComputerGurus:develop
WalkthroughThe update enhances the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 2
Out of diff range and nitpick comments (1)
src/NetCore.Utilities.Spreadsheet/SpreadsheetColumnAttribute.cs (1)
Line range hint
22-22: Add XML documentation for theformulaparameter in the constructor to match other parameters.+ /// <param name="formula">Sets the formula for the column.</param>
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- src/NetCore.Utilities.Spreadsheet.Tests/NetCore.Utilities.Spreadsheet.Tests.csproj (1 hunks)
- src/NetCore.Utilities.Spreadsheet.Tests/OpenXmlSpreadsheetGeneratorTests.cs (2 hunks)
- src/NetCore.Utilities.Spreadsheet.Tests/SampleExportRecord.cs (1 hunks)
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.Designer.cs (1 hunks)
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.cs-CZ.resx (1 hunks)
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.resx (1 hunks)
- src/NetCore.Utilities.Spreadsheet/SpreadsheetColumnAttribute.cs (1 hunks)
- src/NetCore.Utilities.Spreadsheet/TypeDiscoverer.cs (2 hunks)
Files skipped from review due to trivial changes (3)
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.Designer.cs
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.cs-CZ.resx
- src/NetCore.Utilities.Spreadsheet.Tests/TestResources.resx
Additional Context Used
GitHub Check Runs (1)
Validate Build success (7)
src/NetCore.Utilities.Spreadsheet/SpreadsheetColumnAttribute.cs: [warning] 22-22:
Parameter 'formula' has no matching param tag in the XML comment for 'SpreadsheetColumnAttribute.SpreadsheetColumnAttribute(string, float, bool, string, string)' (but other parameters do)
src/NetCore.Utilities.Spreadsheet/SpreadsheetColumnAttribute.cs: [warning] 22-22:
Parameter 'formula' has no matching param tag in the XML comment for 'SpreadsheetColumnAttribute.SpreadsheetColumnAttribute(string, float, bool, string, string)' (but other parameters do)
Additional comments not posted (5)
src/NetCore.Utilities.Spreadsheet.Tests/NetCore.Utilities.Spreadsheet.Tests.csproj (2)
32-41: The configuration for embedded resources is correctly set up for localization support.
43-49: The compilation settings forTestResources.Designer.csare correctly configured to ensure proper association withTestResources.resx.src/NetCore.Utilities.Spreadsheet/SpreadsheetColumnAttribute.cs (2)
57-57: TheResourceFileTypeproperty is correctly implemented to support localization.
62-62: TheResourceKeyproperty is correctly implemented to support localization.src/NetCore.Utilities.Spreadsheet.Tests/SampleExportRecord.cs (1)
121-125: TheLocalizedSampleExportRecordclass is correctly implemented to test the localization functionality using the new properties.
| if (sca.ResourceFileType != null && string.IsNullOrWhiteSpace(sca.ResourceKey) == false) | ||
| { | ||
| ResourceManager rm = new(sca.ResourceFileType); | ||
| propName = rm.GetString(sca.ResourceKey) ?? propName; | ||
| } |
There was a problem hiding this comment.
The implementation for retrieving localized property names using ResourceManager is robust. Consider adding error handling for potential exceptions from ResourceManager.GetString.
+ try
+ {
+ propName = rm.GetString(sca.ResourceKey) ?? propName;
+ }
+ catch (Exception ex)
+ {
+ // Handle exception (e.g., log it)
+ }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.
| if (sca.ResourceFileType != null && string.IsNullOrWhiteSpace(sca.ResourceKey) == false) | |
| { | |
| ResourceManager rm = new(sca.ResourceFileType); | |
| propName = rm.GetString(sca.ResourceKey) ?? propName; | |
| } | |
| if (sca.ResourceFileType != null && string.IsNullOrWhiteSpace(sca.ResourceKey) == false) | |
| { | |
| ResourceManager rm = new(sca.ResourceFileType); | |
| try | |
| { | |
| propName = rm.GetString(sca.ResourceKey) ?? propName; | |
| } | |
| catch (Exception ex) | |
| { | |
| // Handle exception (e.g., log it) | |
| } | |
| } |
| [Fact] | ||
| public void CreateSheet_With_Localized_ColumnName() | ||
| { | ||
| var configuration = new SpreadsheetConfiguration<LocalizedSampleExportRecord> | ||
| { | ||
| WorksheetName = "TestSheet", | ||
| ExportData = new List<LocalizedSampleExportRecord>() | ||
| { | ||
| new() | ||
| { | ||
| RecordTitle = "test record" | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| CultureInfo.CurrentUICulture = CultureInfo.GetCultureInfo("cs-CZ"); | ||
|
|
||
| using var ms = new MemoryStream(); | ||
| var result = _spreadsheetGenerator.CreateSingleSheetSpreadsheet(ms, configuration); | ||
|
|
||
| // ms.Seek(0, SeekOrigin.Begin); | ||
| // File.WriteAllBytes(@"d:\dcore\test.xlsx", ms.ToArray()); | ||
|
|
||
| result.Should().BeTrue(); | ||
| ms.Should().NotHaveLength(0); | ||
| } |
There was a problem hiding this comment.
The test method CreateSheet_With_Localized_ColumnName is well-implemented for generating a spreadsheet with a localized column name. Consider adding a verification step to ensure that the column names are correctly localized in the resulting file.
Would you like assistance in implementing this verification step?
There is a simple test included but I don't know how to verify in the test that the column in the result file is correctly localized.
I have manually verified this and am already using it successfully in my project.
Thanks
Summary by CodeRabbit
New Features
Tests
Localization