🐛 Allow Only/Exclude/Indifferent filters for appeal status #170
Conversation
|
Your Render PR Server URL is https://ozone-staging-pr-170.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-crb2bprv2p9s738a4l7g. |
| if (appealed) { | ||
| queryParams.appealed = appealed | ||
| // If not specifically set to true but there is a value, we can default to false | ||
| // No value will pass undefined which will be ignored | ||
| queryParams.appealed = appealed === 'true' | ||
| } |
There was a problem hiding this comment.
I wonder if an unknown value would be better suited to be interpreted as "unset" rather than false, and some of the logic above seems to handle it that way.
There was a problem hiding this comment.
E.g. something like:
if (appealed === 'true') {
queryParams.appealed = true
} else if (appealed === 'false') {
queryParams.appealed = false
}There was a problem hiding this comment.
hmm... I think I see what you mean but params.get returns null and setting it to an "unknown" value feels odd and could only be needed if we ever give this more meaning in which case we'd need another touchpoint here anyways. so I'll leave it as is for now but happy to discuss more if you feel strongly.
There was a problem hiding this comment.
The main thing I was noticing is that in some logic (mostly the display logic) an unknown value was treated as false and in other logic it was treated as unset. So it looked like a consistency issue, e.g. the UI might display something different than what the server was receiving.
There was a problem hiding this comment.
Trying to illustrate better! I expect the current behavior is like this:
?appealed=- UI shows "Appeals"
- 👍 Server doesn't filter based on appeal
?appealed=true- UI shows "Only Appeals"
- 👍 Server services "only appeals"
?appealed=false- UI shows "No Appeals"
- 👍 Server services "no appeals"
?appealed=unknown- UI shows "Appeals"
⚠️ Server services "no appeals"
In the last case there I would expect the server not to filter based on appeals, which both would match the UI and seems like a sensible way to interpret the unknown value.
There was a problem hiding this comment.
makes sense. will get it wired up in the next PR! thanks
Co-authored-by: devin ivy <[email protected]>
* 🐛 Allow Only/Exclude/Indifferent filters for appeal status * 🐛 Fix page title * Line up logic with surrounding code Co-authored-by: devin ivy <[email protected]> --------- Co-authored-by: devin ivy <[email protected]>
Depends on bluesky-social/atproto#2777
Screen.Recording.2024-09-02.at.22.48.35.mov