Skip to content

Fix DAG export bug #223

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

Merged
merged 4 commits into from
May 30, 2025
Merged

Fix DAG export bug #223

merged 4 commits into from
May 30, 2025

Conversation

ezraporter
Copy link
Collaborator

Description

This PR fixes a bug causing users to receive an unclear error message when trying to export data access group information when they don't have the correct privileges.

Proposed Changes

Screenshots

Screenshot 2025-05-30 at 9 57 04 AM

Issue Addressed

Closes #[provide issue number to link here]

PR Checklist

Before submitting this PR, please check and verify below that the submission meets the below criteria:

  • New/revised functions have associated tests
  • New/revised functions that update downstream outputs have associated static testing files (.RDS) updated under inst/testdata/create_test_data.R
  • New/revised functions use appropriate naming conventions
  • New/revised functions don't repeat code
  • Code changes are less than 250 lines total
  • Issues linked to the PR using GitHub's list of keywords
  • The appropriate reviewer is assigned to the PR
  • The appropriate developers are assigned to the PR
  • Pre-release package version incremented using usethis::use_version()

Code Review

This section to be used by the reviewer and developers during Code Review after PR submission

Code Review Checklist

  • I checked that new files follow naming conventions and are in the right place
  • I checked that documentation is complete, clear, and without typos
  • I added/edited comments to explain "why" not "how"
  • I checked that all new variable and function names follow naming conventions
  • I checked that new tests have been written for key business logic and/or bugs that this PR fixes
  • I checked that new tests address important edge cases

@ezraporter ezraporter requested a review from rsh52 May 30, 2025 14:10
@ezraporter ezraporter self-assigned this May 30, 2025
Copy link
Collaborator

@rsh52 rsh52 left a comment

Choose a reason for hiding this comment

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

Thanks for adding this, all looks good to me. When you go to submit, just be sure to update NEWS.md to reflect these two bug fixes.

@@ -31,6 +31,7 @@ jobs:
REDCAPTIDIER_LONGITUDINAL_DAG_API: ${{ secrets.REDCAPTIDIER_LONGITUDINAL_DAG_API }}
REDCAPTIDIER_MIXED_STRUCTURE_API: ${{ secrets.REDCAPTIDIER_MIXED_STRUCTURE_API }}
REDCAPTIDIER_MDC_API: ${{ secrets.REDCAPTIDIER_MDC_API }}
REDCAPTIDIER_DAG_ACCESS_API: ${{ secrets.REDCAPTIDIER_DAG_ACCESS_API }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Annoying to add a whole other database and secret for this but c'est la vie.

) {
condition$info <- c(
"!" = "You do not have sufficient privileges to export data access groups.",
"i" = "Set `export_data_access_groups = FALSE` if you do not intend to export data access groups."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect!

@ezraporter ezraporter merged commit 328c7d0 into main May 30, 2025
4 checks passed
@ezraporter ezraporter deleted the fix-dag-export-bug branch May 30, 2025 16:27
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