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

Jump to bottom

BED-4537: Create Share endpoint #775

Merged
merged 6 commits into from
Aug 27, 2024
Merged

BED-4537: Create Share endpoint #775

merged 6 commits into from
Aug 27, 2024

Conversation

ALCooper12
Copy link
Contributor

@ALCooper12 ALCooper12 commented Aug 6, 2024 •
edited
Loading

Description

  • Created a new ShareSavedQueries endpoint
  • Added unit tests
  • Updated openapi docs

Motivation and Context

  • Ticket: BED-4537
  • Subtask above is from BED-4534

Why is this change required? What problem does it solve?

  • This newly created endpoint will allow users to have the ability to share a query with others, or set it as public

How Has This Been Tested?

  • 3 successfully expected api calls via Bruno + database changes
Screenshot 2024-08-05 at 5 56 33 PM Screenshot 2024-08-05 at 5 57 53 PM Screenshot 2024-08-05 at 5 58 36 PM Screenshot 2024-08-05 at 5 59 55 PM Screenshot 2024-08-05 at 6 00 38 PM

OpenAPI preview (but please check on how this looks like for you!)
Screenshot 2024-08-05 at 6 05 56 PM

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

@ALCooper12 ALCooper12 added documentation Improvements or additions to documentation enhancement New feature or request api A pull request containing changes affecting the API code. labels Aug 6, 2024
@ALCooper12 ALCooper12 self-assigned this Aug 6, 2024
mvlipka
mvlipka approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@mvlipka mvlipka left a comment

Choose a reason for hiding this comment

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

Looks good!
@irshadaj I'd still like another pair of eyes on it since I'm a bit bias here 😄

mistahj67
mistahj67 requested changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

I had a number of questions as well that we DM'd and it looks like there's some additional work on the table still.

cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
irshadaj
irshadaj requested changes Aug 6, 2024
View reviewed changes
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
mistahj67
mistahj67 reviewed Aug 8, 2024
View reviewed changes
.github/workflows/publish.yml Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/saved_queries.go Outdated Show resolved Hide resolved
cmd/api/src/database/saved_queries_permissions.go Outdated Show resolved Hide resolved
cmd/api/src/database/saved_queries_permissions.go Show resolved Hide resolved
@ALCooper12 ALCooper12 mentioned this pull request Aug 20, 2024
Closed
3 tasks
@mistahj67 mistahj67 force-pushed the BED-4537 branch 2 times, most recently from cd67c46 to 69e5213 Compare August 21, 2024 19:42
@mistahj67
created ShareSavedQueries endpoint and updated openapi docs
5f7a1f5 
BED-4537 db methods by Mike

BED-4537 added database functionality to query for all scopes, integration tests for that

BED-4537 linter fix for error

BED-4537 added ability to delete saved query permissions and integration tests for it

BED-4537 added ability to delete saved query permissions, bulk create saved query permissions, and integration tests

BED-4537 forgot to push batch changes, whoops!

BED-4537 fix linting error

Addressed PR feedback, corrected yaml files and unit tests

Address PR feedback and optimization changes

Refactored control flow logic, added TONS of unit tests, added some integration tests, and altered some database functions

Refactored logic, added unit/integration tests, handled merge conflicts

Addressed previous PR feedback and adjusted unit tests

Corrected openapi stuff

Changed the endpoint url

More openapi corrections and file name change

Addressed PR feedback and fixed unit/integration tests
@mistahj67 mistahj67 force-pushed the BED-4537 branch 3 times, most recently from a038e15 to 4643ced Compare August 23, 2024 22:53
mistahj67
mistahj67 approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Looks solid! I had a few small changes to shore up testing / cleaning but don't want to block merging 🚀 once you've tackled those. Great job and thanks for your patience while we iterated through the scenarios and logic, it came out clean! 🧼

cmd/api/src/database/saved_queries_permissions.go Outdated Show resolved Hide resolved
cmd/api/src/database/saved_queries_permissions.go Outdated Show resolved Hide resolved
cmd/api/src/database/saved_queries_permissions_test.go Show resolved Hide resolved
cmd/api/src/database/saved_queries_permissions_test.go Outdated Show resolved Hide resolved
mistahj67
mistahj67 approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

🚀

@ALCooper12 ALCooper12 merged commit af12ed0 into main Aug 27, 2024
4 checks passed
@ALCooper12 ALCooper12 deleted the BED-4537 branch August 27, 2024 21:47
@github-actions github-actions bot locked and limited conversation to collaborators Aug 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mvlipka mvlipka mvlipka approved these changes

@sircodemane sircodemane sircodemane left review comments

@irshadaj irshadaj irshadaj approved these changes

@mistahj67 mistahj67 mistahj67 approved these changes

Assignees

@ALCooper12 ALCooper12

Labels
api A pull request containing changes affecting the API code. documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ALCooper12 @mvlipka @sircodemane @irshadaj @mistahj67