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

Download Enbridge bills #201

Merged
merged 5 commits into from
Oct 28, 2024
Merged

Download Enbridge bills #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b7b637c
Download Enbridge Bills
rprouse Oct 27, 2024
9cf07f6
Fixes based on testing
rprouse Oct 27, 2024
46de312
Limit to downloading a specified number of months
rprouse Oct 27, 2024
8bc0e9a
Install playwright
rprouse Oct 27, 2024
0fb09ae
Format the date
rprouse Oct 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Guppi.Console/Guppi.Console.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<PackageProjectUrl>https://github.com/rprouse/guppi</PackageProjectUrl>
<RepositoryUrl>https://github.com/rprouse/guppi</RepositoryUrl>
<PackageId>dotnet-guppi</PackageId>
<Version>6.3.0</Version>
<Version>6.3.1</Version>
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Version changes should be documented in a changelog

Please create a CHANGELOG.md file in the root directory to track version changes. The new version 6.3.1 should be documented with:

  • The release date
  • New features (Enbridge bill download functionality)
  • Any bug fixes or improvements
  • Breaking changes (if any)
🔗 Analysis chain

Version bump looks appropriate.

The increment from 6.3.0 to 6.3.1 aligns with semantic versioning for a patch release containing new features and fixes.

Let's verify the changelog is updated:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if CHANGELOG.md exists and contains the new version
# Expected: Entry for version 6.3.1 should exist

if [ -f CHANGELOG.md ]; then
    echo "Checking CHANGELOG.md for version 6.3.1..."
    rg -A 5 "## \[6\.3\.1\]" CHANGELOG.md
else
    echo "CHANGELOG.md not found. Please ensure version changes are documented."
fi

Length of output: 174

<PackAsTool>true</PackAsTool>
<ToolCommandName>guppi</ToolCommandName>
<PackageOutputPath>./nupkg</PackageOutputPath>
Expand Down
2 changes: 1 addition & 1 deletion Guppi.Console/Properties/launchSettings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"profiles": {
"Guppi.Console": {
"commandName": "Project",
"commandLineArgs": "bills alectra"
"commandLineArgs": "bills all"
}
}
}
40 changes: 35 additions & 5 deletions Guppi.Console/Skills/BillsSkill.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,39 @@ internal class BillsSkill(IBillService service) : ISkill

public IEnumerable<Command> GetCommands()
{
var option = new Option<int>(new string[] { "--months", "-m" }, () => 1, "The number of months of bills to download.");

var all = new Command("all", "Download bills from all providers")
{
option
};
all.Handler = CommandHandler.Create(async (int months) => await DownloadAllBills(months));

var alectra = new Command("alectra", "Download bills from Alectra")
{
option
};
alectra.Handler = CommandHandler.Create(async (int months) => await DownloadAlectraBills(months));

var enbridge = new Command("enbridge", "Download bills from Enbridge")
{
option
};
alectra.Handler = CommandHandler.Create(async () => await DownloadAlectraBills());
enbridge.Handler = CommandHandler.Create(async (int months) => await DownloadEnbridgeBills(months));

var configure = new Command("configure", "Configures the Bill provider");
configure.AddAlias("config");
configure.Handler = CommandHandler.Create(() => Configure());

var install = new Command("install", "Installs Playwright for the Bill provider");
install.Handler = CommandHandler.Create(() => _service.InstallPlaywright());

var command = new Command("bills", "Download bills from online")
{
all,
alectra,
enbridge,
install,
configure
};
command.AddAlias("billing");
Expand All @@ -36,13 +57,22 @@ public IEnumerable<Command> GetCommands()
return new List<Command> { command };
}

private async Task DownloadAlectraBills()
{
private async Task DownloadAllBills(int months) =>
await DownloadBills(":spiral_notepad: Download Bills", months, _service.DownloadAllBills);

private async Task DownloadAlectraBills(int months) =>
await DownloadBills(":high_voltage: Alectra Bills", months, _service.DownloadAlectraBills);

private async Task DownloadEnbridgeBills(int months) =>
await DownloadBills(":chart_increasing: Enbridge Bills", months, _service.DownloadEnbridgeBills);

private static async Task DownloadBills(string title, int months, Func<int, Task> downloader)
{
try
{
AnsiConsoleHelper.TitleRule(":high_voltage: Alectra Bills");
AnsiConsoleHelper.TitleRule(title);

await _service.DownloadAlectraBills();
await downloader(months);

AnsiConsoleHelper.Rule("white");
}
Expand Down
5 changes: 4 additions & 1 deletion Guppi.Core/Interfaces/Services/IBillService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ namespace Guppi.Core.Interfaces.Services;

public interface IBillService
{
Task DownloadAlectraBills();
Task DownloadAllBills(int months);
Task DownloadAlectraBills(int months);
Task DownloadEnbridgeBills(int months);
Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Parameter validation needed for months parameter

Looking at the implementation in BillService.cs, there is no validation for the months parameter in any of the download methods. This could lead to potential issues with:

  • Negative values
  • Unreasonably large values that could overload the system
  • Zero values that would result in no downloads

Suggested locations that need validation:

  • Guppi.Core/Services/BillService.cs: Add validation in the public methods DownloadAllBills, DownloadAlectraBills, and DownloadEnbridgeBills
  • Guppi.Core/Interfaces/Services/IBillService.cs: Add XML documentation to specify valid ranges for the months parameter
🔗 Analysis chain

Consider adding parameter validation.

The months parameter should be validated to ensure it's within acceptable ranges. Consider:

  1. Adding a custom attribute or documentation to specify valid ranges
  2. Implementing validation in the service implementation

Let's check if there's any validation in the implementation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the service implementation to verify parameter validation
ast-grep --pattern 'class $_ : IBillService {
  $$$
  async Task DownloadAllBills(int months) {
    $$$
  }
  $$$
}'

Length of output: 118

Script:

#!/bin/bash
# Let's try a broader search for implementations of IBillService
rg "class.*IBillService" -A 5

# Also search for any implementation of these specific methods
rg "DownloadAllBills|DownloadAlectraBills|DownloadEnbridgeBills" -A 3

Length of output: 7619

void InstallPlaywright();
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider separating infrastructure concerns.

InstallPlaywright() appears to be an infrastructure concern that violates the Interface Segregation Principle. Consider moving it to a separate interface like IPlaywrightInstaller to maintain better separation of concerns.

Example refactor:

public interface IPlaywrightInstaller
{
    void InstallPlaywright();
}

public interface IBillService
{
    Task DownloadAllBills(int months);
    Task DownloadAlectraBills(int months);
    Task DownloadEnbridgeBills(int months);
    void Configure();
}

void Configure();
}
48 changes: 48 additions & 0 deletions Guppi.Core/Providers/WorkbookProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System.Collections.Generic;
using System.IO;
using ClosedXML.Excel;

namespace Guppi.Core.Providers;

public class WorkbookProvider
{
readonly string _filename;
readonly IXLWorkbook _workbook;
readonly IXLWorksheet _worksheet;
int row = 0;
Comment on lines +7 to +12
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider implementing an interface for better testability.

The WorkbookProvider class would benefit from having an interface definition (e.g., IWorkbookProvider). This would make it easier to mock in unit tests and follow the dependency inversion principle.

+public interface IWorkbookProvider
+{
+    void AddRow(IEnumerable<string> values);
+    void Save();
+}

-public class WorkbookProvider
+public class WorkbookProvider : IWorkbookProvider

Committable suggestion was skipped due to low confidence.


/// <summary>
/// Creates a new instance of the WorksheetProvider
/// </summary>
/// <param name="filename">The filename for the workbook</param>
/// <param name="sheet">The name of the worksheet</param>
/// <param name="delete">Deletes and recreates the workbook if it exists</param>
public WorkbookProvider(string filename, string sheet, IEnumerable<string> headers, bool delete = true)
{
_filename = filename;

if (delete && File.Exists(_filename))
{
File.Delete(_filename);
}

_workbook = new XLWorkbook();
_worksheet = _workbook.Worksheets.Add("Bills");

AddRow(headers);
}
Comment on lines +14 to +33
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Multiple improvements needed in constructor implementation.

  1. The sheet parameter is ignored, and "Bills" is hardcoded in line 30
  2. Missing parameter validation
  3. Incomplete XML documentation

Apply these improvements:

 /// <summary>
-/// Creates a new instance of the WorksheetProvider
+/// Creates a new instance of the WorkbookProvider for managing Excel workbooks
 /// </summary>
 /// <param name="filename">The filename for the workbook</param>
-/// <param name="sheet">The name of the worksheet</param>
+/// <param name="sheet">The name of the worksheet to create</param>
+/// <param name="headers">The column headers to add as the first row</param>
 /// <param name="delete">Deletes and recreates the workbook if it exists</param>
+/// <exception cref="ArgumentNullException">Thrown when filename, sheet, or headers is null</exception>
+/// <exception cref="ArgumentException">Thrown when headers collection is empty</exception>
 public WorkbookProvider(string filename, string sheet, IEnumerable<string> headers, bool delete = true)
 {
+    ArgumentNullException.ThrowIfNull(filename);
+    ArgumentNullException.ThrowIfNull(sheet);
+    ArgumentNullException.ThrowIfNull(headers);
+    
+    if (!headers.Any())
+        throw new ArgumentException("Headers collection cannot be empty", nameof(headers));
+
     _filename = filename;

     if (delete && File.Exists(_filename))
     {
         File.Delete(_filename);
     }

     _workbook = new XLWorkbook();
-    _worksheet = _workbook.Worksheets.Add("Bills");
+    _worksheet = _workbook.Worksheets.Add(sheet);

     AddRow(headers);
 }
📝 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.

Suggested change
/// <summary>
/// Creates a new instance of the WorksheetProvider
/// </summary>
/// <param name="filename">The filename for the workbook</param>
/// <param name="sheet">The name of the worksheet</param>
/// <param name="delete">Deletes and recreates the workbook if it exists</param>
public WorkbookProvider(string filename, string sheet, IEnumerable<string> headers, bool delete = true)
{
_filename = filename;
if (delete && File.Exists(_filename))
{
File.Delete(_filename);
}
_workbook = new XLWorkbook();
_worksheet = _workbook.Worksheets.Add("Bills");
AddRow(headers);
}
/// <summary>
/// Creates a new instance of the WorkbookProvider for managing Excel workbooks
/// </summary>
/// <param name="filename">The filename for the workbook</param>
/// <param name="sheet">The name of the worksheet to create</param>
/// <param name="headers">The column headers to add as the first row</param>
/// <param name="delete">Deletes and recreates the workbook if it exists</param>
/// <exception cref="ArgumentNullException">Thrown when filename, sheet, or headers is null</exception>
/// <exception cref="ArgumentException">Thrown when headers collection is empty</exception>
public WorkbookProvider(string filename, string sheet, IEnumerable<string> headers, bool delete = true)
{
ArgumentNullException.ThrowIfNull(filename);
ArgumentNullException.ThrowIfNull(sheet);
ArgumentNullException.ThrowIfNull(headers);
if (!headers.Any())
throw new ArgumentException("Headers collection cannot be empty", nameof(headers));
_filename = filename;
if (delete && File.Exists(_filename))
{
File.Delete(_filename);
}
_workbook = new XLWorkbook();
_worksheet = _workbook.Worksheets.Add(sheet);
AddRow(headers);
}


public void AddRow(IEnumerable<string> values)
{
row++;
int col = 1;
foreach (string value in values) {
_worksheet.Cell(row, col++).Value = value;
}
}
Comment on lines +35 to +42
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation and documentation to AddRow method.

The method needs parameter validation and XML documentation. It should also ensure the number of values matches the number of columns.

Apply these improvements:

+/// <summary>
+/// Adds a new row of values to the worksheet
+/// </summary>
+/// <param name="values">The values to add to the row</param>
+/// <exception cref="ArgumentNullException">Thrown when values is null</exception>
+/// <exception cref="ArgumentException">Thrown when number of values doesn't match the number of columns</exception>
 public void AddRow(IEnumerable<string> values)
 {
+    ArgumentNullException.ThrowIfNull(values);
+    
+    var valuesList = values.ToList();
+    var columnCount = _worksheet.Row(1).CellsUsed().Count();
+    
+    if (valuesList.Count != columnCount)
+        throw new ArgumentException($"Number of values ({valuesList.Count}) must match number of columns ({columnCount})", nameof(values));
+
     row++;
     int col = 1;
-    foreach (string value in values) {
+    foreach (string value in valuesList) {
         _worksheet.Cell(row, col++).Value = value;
     }
 }
📝 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.

Suggested change
public void AddRow(IEnumerable<string> values)
{
row++;
int col = 1;
foreach (string value in values) {
_worksheet.Cell(row, col++).Value = value;
}
}
/// <summary>
/// Adds a new row of values to the worksheet
/// </summary>
/// <param name="values">The values to add to the row</param>
/// <exception cref="ArgumentNullException">Thrown when values is null</exception>
/// <exception cref="ArgumentException">Thrown when number of values doesn't match the number of columns</exception>
public void AddRow(IEnumerable<string> values)
{
ArgumentNullException.ThrowIfNull(values);
var valuesList = values.ToList();
var columnCount = _worksheet.Row(1).CellsUsed().Count();
if (valuesList.Count != columnCount)
throw new ArgumentException($"Number of values ({valuesList.Count}) must match number of columns ({columnCount})", nameof(values));
row++;
int col = 1;
foreach (string value in valuesList) {
_worksheet.Cell(row, col++).Value = value;
}
}


public void Save()
{
_workbook.SaveAs(_filename);
}
Comment on lines +44 to +47
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling, documentation, and implement IDisposable.

The class should implement IDisposable to properly clean up ClosedXML resources. The Save method needs error handling and documentation.

Apply these improvements:

-public class WorkbookProvider : IWorkbookProvider
+public class WorkbookProvider : IWorkbookProvider, IDisposable
 {
     // ... existing code ...

+    /// <summary>
+    /// Saves the workbook to the specified filename
+    /// </summary>
+    /// <exception cref="IOException">Thrown when the file cannot be saved</exception>
     public void Save()
     {
-        _workbook.SaveAs(_filename);
+        try
+        {
+            _workbook.SaveAs(_filename);
+        }
+        catch (Exception ex)
+        {
+            throw new IOException($"Failed to save workbook to {_filename}", ex);
+        }
     }
+
+    public void Dispose()
+    {
+        _workbook?.Dispose();
+    }
 }
📝 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.

Suggested change
public void Save()
{
_workbook.SaveAs(_filename);
}
public class WorkbookProvider : IWorkbookProvider, IDisposable
{
/// <summary>
/// Saves the workbook to the specified filename
/// </summary>
/// <exception cref="IOException">Thrown when the file cannot be saved</exception>
public void Save()
{
try
{
_workbook.SaveAs(_filename);
}
catch (Exception ex)
{
throw new IOException($"Failed to save workbook to {_filename}", ex);
}
}
public void Dispose()
{
_workbook?.Dispose();
}
}

}
Loading
Loading