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.

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

BCDA-5355: Disable ALR export endpoints #853

Merged
merged 5 commits into from
Jun 23, 2023
Merged

BCDA-5355: Disable ALR export endpoints #853

merged 5 commits into from
Jun 23, 2023

Conversation

kyeah
Copy link
Contributor

@kyeah kyeah commented Jun 22, 2023

🎫 Ticket

https://jira.cms.gov/browse/BCDA-5355

🛠 Changes

  • Disable ALR endpoints in deployed environments

ℹ️ Context for reviewers

The ALR export endpoints were never completed and officially released, and they should be disabled to prevent confusion.

Since the default is false, I didn't add env vars for ENABLE_ALR_ENDPOINTS in our deployed environments. However, if folks feel we should have those, I can add that separately.

✅ Acceptance Validation

Verified locally:

bcda-app: curl -X GET 'http://localhost:3000/api/v2/alr/$export' \                                                                                                                  
                        -H "accept: application/fhir+json" \
                        -H "Prefer: respond-async" \
                        -H "Authorization: Bearer $token"
404 page not found

bcda-app: curl -X GET 'http://localhost:3000/api/v1/alr/$export' \
                        -H "accept: application/fhir+json" \ 
                        -H "Prefer: respond-async" \
                        -H "Authorization: Bearer $token"
404 page not found

🔒 Security Implications

  • This PR adds a new software dependency or dependencies.
  • This PR modifies or invalidates one or more of our security controls.
  • This PR stores or transmits data that was not stored or transmitted before.
  • This PR requires additional review of its security implications for other reasons.

@@ -54,7 +54,9 @@ func NewAPIRouter() http.Handler {

r.Route("/api/v1", func(r chi.Router) {
r.With(append(commonAuth, requestValidators...)...).Get(m.WrapHandler("/Patient/$export", v1.BulkPatientRequest))
r.With(append(commonAuth, requestValidators...)...).Get(m.WrapHandler("/alr/$export", v1.ALRRequest))
if conf.GetEnv("ENABLE_ALR_ENDPOINTS") == "true" {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice and simple! LGTM.

@kyeah kyeah merged commit ff3bbd0 into master Jun 23, 2023
@kyeah kyeah deleted the kev/alr-disable branch June 23, 2023 16:55
laurenkrugen-navapbc pushed a commit that referenced this pull request Jul 11, 2023
## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-5355

## 🛠 Changes

- Disable ALR endpoints in deployed environments

## ℹ️ Context for reviewers

The ALR export endpoints were never completed and officially released,
and they should be disabled to prevent confusion.

Since the default is false, I didn't add env vars for
ENABLE_ALR_ENDPOINTS in our deployed environments. However, if folks
feel we should have those, I can add that separately.

## ✅ Acceptance Validation

Verified locally:

```sh
bcda-app: curl -X GET 'http://localhost:3000/api/v2/alr/$export' \                                                                                                                  
                        -H "accept: application/fhir+json" \
                        -H "Prefer: respond-async" \
                        -H "Authorization: Bearer $token"
404 page not found

bcda-app: curl -X GET 'http://localhost:3000/api/v1/alr/$export' \
                        -H "accept: application/fhir+json" \ 
                        -H "Prefer: respond-async" \
                        -H "Authorization: Bearer $token"
404 page not found
```

## 🔒 Security Implications

- [ ] This PR adds a new software dependency or dependencies.
- [ ] This PR modifies or invalidates one or more of our security
controls.
- [ ] This PR stores or transmits data that was not stored or
transmitted before.
- [ ] This PR requires additional review of its security implications
for other reasons.
kyeah added a commit that referenced this pull request Jul 2, 2024
## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-8227

## 🛠 Changes

- Replace completed_job_count with a count of unique job keys (based on
job_key.que_job_id)
- Avoid re-processing ALR job if que job was already processed

## ℹ️ Context

As part of #964, I discovered that the completed_job_count was still
being used as part of ALR jobs. This applies the same fix from the
"normal" export jobs in #962.

Note that many different job keys can be produced by an ALR job. To
simplify the determination of whether a que job was processed, I added a
new "getUniqueJobKeyCount" method.

For some historical context, the ALR endpoints are not enabled in
deployed environments - see #853 for more details. To avoid making a
decision on whether to remove the code entirely, I am applying these
changes for now to remove completed_job_count.

## 🧪 Validation

Unit Testing
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