lix-community/nix-security-tracker
0
0
Fork 0
Code Issues 94 Pull requests 9 Projects Releases Packages Wiki Activity

fix: undo action didn't work anymore #422

Merged
erictapen merged 1 commit from fix-undo into main 2024-12-06 09:47:53 +00:00
Conversation 11 Commits 1 Files changed 6 +81 -87
erictapen commented 2024-12-05 14:54:17 +00:00 (Migrated from github.com)
Copy link

also refactored update_suggestion, this method was never meant to only be called once

Fixes #421

also refactored update_suggestion, this method was never meant to only be called once Fixes #421
fricklerhandwerk commented 2024-12-06 00:57:44 +00:00 (Migrated from github.com)
Copy link

Hm, this doesn't seem to fix it yet. The item only returns temporarily; on reload it's gone (and always stayed in the other state).

Hm, this doesn't seem to fix it yet. The item only returns temporarily; on reload it's gone (and always stayed in the other state).
erictapen commented 2024-12-06 08:48:05 +00:00 (Migrated from github.com)
Copy link

It should work now. It looks like, the undo action never really worked before? 😬

It should work now. It looks like, the undo action never really worked before? :grimacing:
fricklerhandwerk commented 2024-12-06 08:52:27 +00:00 (Migrated from github.com)
Copy link

It looks like, the undo action never really worked before?

Two test engineers walk into a bar...

> It looks like, the undo action never really worked before? Two test engineers walk into a bar...
😆 1
fricklerhandwerk (Migrated from github.com) reviewed 2024-12-06 09:12:40 +00:00
src/website/webview/views.py
@ -529,0 +533,4 @@
# We only have to modify derivations when they are editable
if not (
self.status_filter == CVEDerivationClusterProposal.Status.REJECTED
or undo_status_change
fricklerhandwerk (Migrated from github.com) commented 2024-12-06 08:59:40 +00:00
Copy link

Just a side note, not a blocker, and I didn't review it out when it was added, but: for all intents an purposes JS is stuff on top of Berners-Lee web, which is why it should add its own cludges, not remove those added to pre-emptively reserve space for client-side-scripted workflows.

Just a side note, not a blocker, and I didn't review it out when it was added, but: for all intents an purposes JS is stuff on top of Berners-Lee web, which is why *it should add its own cludges*, not remove those added to pre-emptively reserve space for client-side-scripted workflows.
src/website/webview/views.py
@ -546,2 +587,4 @@
"suggestion": suggestion,
"activity_log": activity_log,
"status_filter": self.status_filter,
# This only matters in a non-JS environment
fricklerhandwerk (Migrated from github.com) commented 2024-12-06 09:12:21 +00:00
Copy link

Why only here and not for the other stati? Or, why the and undo_status_change at all?

Why only here and not for the other stati? Or, why the `and undo_status_change` at all?
erictapen (Migrated from github.com) reviewed 2024-12-06 09:18:36 +00:00
src/website/webview/views.py
@ -546,2 +587,4 @@
"suggestion": suggestion,
"activity_log": activity_log,
"status_filter": self.status_filter,
# This only matters in a non-JS environment
erictapen (Migrated from github.com) commented 2024-12-06 09:18:36 +00:00
Copy link

This ensures, that there is only one way back to the pending state: By clicking the undo button. So this is really only there for double-checking anyway, as there is currently no button to achieve this.

This ensures, that there is only one way back to the pending state: By clicking the undo button. So this is really only there for double-checking anyway, as there is currently no button to achieve this.
erictapen (Migrated from github.com) reviewed 2024-12-06 09:21:52 +00:00
src/website/webview/views.py
@ -529,0 +533,4 @@
# We only have to modify derivations when they are editable
if not (
self.status_filter == CVEDerivationClusterProposal.Status.REJECTED
or undo_status_change
erictapen (Migrated from github.com) commented 2024-12-06 09:21:52 +00:00
Copy link

The problem here is that you can't add form parameters specifically for the htmx request, you can only remove them. This is why there is this awkward no-js flag that is filtered out when using htmx.

The problem here is that you can't add form parameters specifically for the htmx request, you can only remove them. This is why there is this awkward no-js flag that is filtered out when using htmx.
fricklerhandwerk (Migrated from github.com) reviewed 2024-12-06 09:27:28 +00:00
src/website/webview/views.py
@ -546,2 +587,4 @@
"suggestion": suggestion,
"activity_log": activity_log,
"status_filter": self.status_filter,
# This only matters in a non-JS environment
fricklerhandwerk (Migrated from github.com) commented 2024-12-06 09:27:27 +00:00
Copy link
        # since we want the queue to shrink monotonically,
        # there's no UI for returning a suggestion back to pending state,
        # but this is an additional safeguard to prevent that from happening
        #                                vvvvvvvvvvvvvvvvvv
        elif new_status == "pending" and undo_status_change:
```suggestion # since we want the queue to shrink monotonically, # there's no UI for returning a suggestion back to pending state, # but this is an additional safeguard to prevent that from happening # vvvvvvvvvvvvvvvvvv elif new_status == "pending" and undo_status_change: ```
fricklerhandwerk (Migrated from github.com) reviewed 2024-12-06 09:28:43 +00:00
src/website/webview/views.py
@ -546,2 +587,4 @@
"suggestion": suggestion,
"activity_log": activity_log,
"status_filter": self.status_filter,
# This only matters in a non-JS environment
fricklerhandwerk (Migrated from github.com) commented 2024-12-06 09:28:43 +00:00
Copy link

Please always answer why-questions in comments, otherwise that knowledge will be lost, and eternal cargo cult ensues (severely exerbated by not having tests)

Please always answer why-questions in comments, otherwise that knowledge will be lost, and eternal cargo cult ensues (severely exerbated by not having tests)
erictapen (Migrated from github.com) reviewed 2024-12-06 09:33:10 +00:00
src/website/webview/views.py
@ -546,2 +587,4 @@
"suggestion": suggestion,
"activity_log": activity_log,
"status_filter": self.status_filter,
# This only matters in a non-JS environment
erictapen (Migrated from github.com) commented 2024-12-06 09:33:10 +00:00
Copy link

Okay, makes sense, I will try to do that 👍

Okay, makes sense, I will try to do that :+1:
fricklerhandwerk (Migrated from github.com) reviewed 2024-12-06 09:45:03 +00:00
src/website/webview/views.py
@ -529,0 +533,4 @@
# We only have to modify derivations when they are editable
if not (
self.status_filter == CVEDerivationClusterProposal.Status.REJECTED
or undo_status_change
fricklerhandwerk (Migrated from github.com) commented 2024-12-06 09:45:03 +00:00
Copy link

Which is why it would make sense to have a separate endpoint. Doing something like

<form method="post" action="" hx-put="/suggestion/{{id}}/state">
    <button type="submit" name="status" value="accepted">Select</button>
    <button type="submit" name="status" value="rejected">Dismiss</button>
</form>

would circumvent all that noise entirely. htmx is absolutely beautiful.

And then we'd have an entirely separate, tidy code path just for the dynamic stuff.

Which is why it would make sense to have a separate endpoint. Doing something like ``` <form method="post" action="" hx-put="/suggestion/{{id}}/state"> <button type="submit" name="status" value="accepted">Select</button> <button type="submit" name="status" value="rejected">Dismiss</button> </form> ``` would circumvent all that noise entirely. htmx is absolutely beautiful. And then we'd have an entirely separate, tidy code path just for the dynamic stuff.
fricklerhandwerk (Migrated from github.com) approved these changes 2024-12-06 09:47:36 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
fricklerhandwerk
Labels
Clear labels   
automation

   
backend

something the web server needs to provide the frontend with

  
bug

Something isn't working

  
contributor experience

   
data

something about quality or quantity of ingested data

  
deployment

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
nice to have

Nice to have, but not necessary for the time being

  
notifications

   
package maintainer

user story for package maintainers

  
performance

   
skin

Anything related to the visual presentation

  
tech debt

Activities that reduce tech debt

  
user story

description or implementation of a workflow

No labels
automation
backend
bug
contributor experience
data
deployment
documentation
duplicate
good first issue
help wanted
nice to have
notifications
package maintainer
performance
skin
tech debt
user story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-community/nix-security-tracker#422
No description provided.
Delete branch "fix-undo"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?