Skip to content

[BACK-4518][BACK-4185] Add patient list export functionality and route. - #243

Open
lostlevels wants to merge 10 commits into
masterfrom
jp-patient-export
Open

[BACK-4518][BACK-4185] Add patient list export functionality and route.#243
lostlevels wants to merge 10 commits into
masterfrom
jp-patient-export

Conversation

@lostlevels

Copy link
Copy Markdown
Contributor

Commit for the patient list export functionality. Defines new type, ExportedPatient, since fields are unique to this view of the patient. Adds appropriate mapping and logic over existing fields.

Commit for the patient list export functionality. Defines new type,
ExportedPatient, since fields are unique to this view of the patient. Adds
appropriate mapping and logic over existing fields.
@lostlevels

Copy link
Copy Markdown
Contributor Author

Ignore @darinkrauss, clicked the wrong button, meant to request @ewollesen .

Comment thread api/patients.go Outdated
Comment thread export/export.go Outdated
Comment thread export/export.go
Comment thread export/export.go
Comment thread export/export.go Outdated
Comment thread export/export.go Outdated
Comment thread export/export.go
if ds == nil || ds.State == "" {
return "NA"
}
inactiveCutoff := now.Add(-time.Hour * 24 * 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one has a const defined in upcoming DID work, so... hopefully I'll remember to refactor this when I rebase onto this work.

Comment thread export/export.go
}
var tagNames []string
for _, tagId := range *tagIds {
if name, ok := tagNamesById[tagId]; ok {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, this will silently ignore any tags found for which we don't know the name... Is that desired? Maybe better to use "" or something..? We probably don't want to error out, but just ignoring that we couldn't find a name for the tag seems wrong too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Plus, any tag not found will result in an empty result in the output, like "Foo," or "Foo,,Bar".

Comment thread export/export.go Outdated
Comment thread export/export.go Outdated

@ewollesen ewollesen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The build fails.

There functions called which are missing definitions.

Have you tested this manually?

@lostlevels

Copy link
Copy Markdown
Contributor Author

The build fails.

There functions called which are missing definitions.

Have you tested this manually?

@ewollesen Forgot to commit a new file. Tested manually.

Comment thread export/pointer.go
Comment thread export/export.go Outdated
@lostlevels
lostlevels requested a review from ewollesen July 15, 2026 16:14
Comment thread export/export.go Outdated
Comment thread patients/repository/repository.go Outdated
Comment thread api/patients.go Outdated
@lostlevels
lostlevels requested a review from ewollesen July 22, 2026 15:04
Comment thread export/export.go Outdated
Comment on lines +142 to +143
"Exported By", // name
"Exported By", // email

@ewollesen ewollesen Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a good reason not to name these Exported By (Name) and Exported By (Email) just for clarity? Isn't that what's in PROD-4185?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No good reason.

Comment thread export/export.go
)

const (
timeFormat = "2006-01-02 03:04 PM"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest adding the time zone here, so it's clearer.

Comment thread export/export.go
Comment thread export/export.go Outdated
Comment thread export/export.go
@lostlevels
lostlevels requested a review from ewollesen July 28, 2026 15:13
ewollesen
ewollesen previously approved these changes Jul 29, 2026

@ewollesen ewollesen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The expiration time calculation is I believe a bug, so should be fixed.

Comment thread export/export.go
}
var tagNames []string
for _, tagId := range *tagIds {
if name, ok := tagNamesById[tagId]; ok {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Plus, any tag not found will result in an empty result in the output, like "Foo," or "Foo,,Bar".

Comment thread export/export.go Outdated
Comment on lines +240 to +241
expiredCutoff := now.Add(-time.Duration(math.Abs(float64(patients.PendingDataSourceExpirationDuration))))
if (ds.State == patients.DataSourceStatePending || ds.State == patients.DataSourceStatePendingReconnect) && ds.ExpirationTime != nil && ds.ExpirationTime.Before(expiredCutoff) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ds.ExpirationTime is the expiration time of the invite. So you should be comparing ds.ExpirationTime.Before(now) to see if its expired, not now - 30 days.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Plus, any tag not found will result in an empty result in the output, like "Foo," or "Foo,,Bar".

I think we discussed this earlier and agreed having no output to indicate the presence of a tag, even if nameless, was preferable to omitting the tag altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants