Skip to content

Conversation

@eightants
Copy link
Contributor

@eightants eightants commented May 19, 2023

Summary

Figma for the new settings show that project settings are saved using a single button. This PR adds that endpoint, and changed much of the logic in the project settings to use the new context that manages settings.

Discussion in Slack about it: https://highlightcorp.slack.com/archives/C01FJL7V630/p1683905993153839

How did you test this change?

Are there any deployment considerations?

@render
Copy link

render bot commented May 19, 2023

@eightants eightants requested review from a team and deltaepsilon and removed request for a team May 19, 2023 23:39
@deltaepsilon deltaepsilon requested review from et and removed request for deltaepsilon May 22, 2023 15:07
@deltaepsilon
Copy link
Contributor

I'm removing myself from the Reviewers list and nominating @et!

I have no idea what this code does.

Copy link
Contributor

@et et left a comment

Choose a reason for hiding this comment

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

🚢

@eightants eightants merged commit f211810 into main May 22, 2023
@eightants eightants deleted the anthony/unified-settings-save branch May 22, 2023 17:55
et added a commit that referenced this pull request Jun 1, 2023
## Summary

<!--
Ideally, there is an attached GitHub issue that will describe the "why".

If relevant, use this section to call out any additional information
you'd like to _highlight_ to the reviewer.
-->

This PR cleans up some unused endpoints that is no longer needed since
we created a consolidated endpoint to save all project settings (#5387).

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

* Confirmed that updating project settings (that exist on the
project_filter_settings table) correctly update
* Added tests

## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

N/A
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.

4 participants