pfs-116 FE only for downloading a report #3
Conversation
Nicholassully
force-pushed
the
pfs-116-fe-report-form
branch
from
5018f15
to
9f111f2
Compare
There was a problem hiding this comment.
This form is meant to select a report type with parameters, which will then generate the report and return it as a file (presuming CSV), but this doesn't appear to do that. We don't have the API for that yet, but that should be what the mock does, so when we do have a solution, we can just plug it in.
Nicholassully
force-pushed
the
pfs-116-fe-report-form
branch
from
cafb428
to
d5c3010
Compare
internal/api/download.go
Outdated
| "net/http" | ||
| ) | ||
|
|
||
| func (c *ApiClient) Download(ctx Context, reportType string, reportJournalType string, reportScheduleType string, reportAccountType string, reportDebtType string, dateField string, dateFromField string, dateToField string, emailField string) error { |
There was a problem hiding this comment.
Field is redundant naming here. Also, dateField needs to be more descriptive to distinguish it from the other date fields.
Nicholassully
force-pushed
the
pfs-116-fe-report-form
branch
from
821da89
to
cfdf923
Compare
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "github.com/opg-sirius-finance-admin/internal/model" |
internal/api/download.go
Outdated
| innerMap := make(map[string]string) | ||
| innerMap[reason] = reason | ||
| validationErrors[reason] = innerMap |
There was a problem hiding this comment.
| innerMap := make(map[string]string) | |
| innerMap[reason] = reason | |
| validationErrors[reason] = innerMap | |
| validationErrors[reason] = map[string]string{reason: reason} |
internal/api/api_client_test.go
Outdated
| ) | ||
|
|
||
| type MockClient struct { | ||
| DoFunc func(req *http.Request) (*http.Response, error) |
There was a problem hiding this comment.
I'd remove this field since you aren't using it
| ) | ||
|
|
||
| type Date struct { | ||
| Time time.Time |
There was a problem hiding this comment.
you could use model.NewDate in tests instead
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
No description provided.