Problem History #39

Elscrux · 2023-11-02T18:35:31Z

First draft of the Problem History which is integrated into the ProgressHandler.
@schweikart If you have ideas on how this feature should look on the website, let me know.

…variable instead

Elscrux · 2023-11-09T11:53:22Z

I added cookies to persist the history over sessions. I hope that this is a good way to do it.
Two more things I thought about:

There could be a delete button for every history entry and a clear all button for the whole history.
I added a way to load solutions instead of solving them another time when the same problem is inside the history. This should probably be changed to be server-side instead. Distinguishing between GET /solution and POST /solve shouldn't be a thing we do here. The server can check if something was already computed once much better than we do here. POST /solve could get a shortcut to return an existing solution to the problem you're trying to solve.

This reverts commit 59786da.

sonarqubecloud · 2023-11-14T18:04:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

Elscrux · 2023-11-14T18:04:52Z

@schweikart I replaced the cookie approach with localstorage and removed the additional dependency

schweikart · 2024-01-22T16:12:04Z

While testing it, clicking on a previous state threw the following error to the console

Access to fetch at 'http://localhost:8080/solution/sat?id=22' from origin 'http://localhost:3000' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

Can you fix this?

(Related conversation: #39 (comment))

schweikart · 2024-01-22T16:22:03Z

~~Rebased onto develop, as I have caused the conflicts with my formatting PR and merged the dependency PR~~

Seems like I messed something up while rebasing. I reset the branch to the state before my rebase attempt

schweikart

Sorry to keep you waiting for so long...

There are a few things I'd like to draw your attention to:

Fetching previous states doesn't work (leading to a preflight CORS error on my machine)
The historyStateName identifier leads to incomplete state restoration for the feature model anomaly solver.
Can you implement a Clear history button? I think this wouldn't be too difficult and help a lot with using and debugging the toolbox.
Also, I've annotated some minor code quality issues that shouldn't be too hard to fix.

I haven't reviewed the code changes from the Dimacs parser removal, as we've already merged (another version of) them in #38. Can you remove them from this branch?

schweikart · 2023-11-22T08:19:39Z

src/components/solvers/History.tsx

+  setContent: (t: T) => void;
+  loadSolution: (id: ProblemTypeSolutionId) => void;


This naming is unconventional for React. I suggest using onRequestSolution for loadSolution to express the event-like character of passing data upwards. I'm unsure what to do with setContent, maybe onRequestContent or onContentChange?

maybe rather a joint event onRequestRollback(problemState) that can be handled by the containing element? Then, this element wouldn't have to deal with the data managed by another element.

schweikart · 2023-11-22T08:21:03Z

src/components/solvers/ProgressHandler.tsx

+   */
+  content: T;


To me, this content name seems a bit unintuitive. Maybe use input?

It's inspired by SolveRequest's naming which has a field called requestContent - probably not the best naming either so we might end up changing that too.
In any case, I've changed this to problemInput. What do you think?

schweikart · 2023-11-22T08:24:28Z

src/components/solvers/ProgressHandler.tsx

+  /**
+   * Ids of the solutions of the problem input per problem type
+   */
+  solutionIds: ProblemTypeSolutionId;


Maybe just use a Map here?

(or was there a problem with using maps in serialization? I can't quite remember)

There is a serialization issue here but not the one we had when passing a map to the server.
When storing something in the local storage, we just store a string. When deserializing the string, the type information of the map won't persist because of TypeScript limitations which causes that you can't call any functions on the object, as only the primitive object data is loaded but not the type information which are lost.

schweikart · 2023-11-22T08:26:11Z

src/components/solvers/ProgressHandler.tsx

+      <History
+        problemStates={problemStates}
+        loadSolution={loadSolution}
+        setContent={props.setProblemInput}
+      />


Why is the history part of the progress handler? Shouldn't this be independent from each other? Similarly, I don't think problemState data should be managed by the progress handler

Iirc this was a compromise as I didn't find a good structure to be able to load solutions (which is handled by the ProgressHandler) from the History without requiring a lot of boilerplate code in all the problem masks.
I'm not sure what the best way to resolve these kinds of communication problems is in React. Do you know any patterns or techniques to do that properly there? Some event-driven thing or so?

src/components/solvers/SAT/TextArea.tsx

schweikart · 2024-01-23T08:39:18Z

src/api/ToolboxAPI.ts

+}
+
+export async function fetchSolution(problemType: string, solutionId: number) {
+  return fetch(`${baseUrl()}/solution/${problemType}?id=${solutionId}`, {


This code seems to throw a preflight CORS error when a solution is clicked

Fixed by this toolbox PR that I forgot to make
ProvideQ/toolbox-server#67

schweikart · 2024-01-23T08:47:51Z

src/components/solvers/ProgressHandler.tsx

 }

 export const ProgressHandler = <T extends {}>(
  props: ProgressHandlerProps<T>
 ) => {
+  const historyItemName: string = `problemStates-${props.problemTypes}`;


This identification scheme breaks for our Feature Model Anomaly solver. If I run a solve request only for the DEAD anomaly type, this state is gone after refreshing the page.

Maybe store results separately for separate problem types?

Good catch! I think storying them separately would be good but it also creates some new questions. One in particular, that I don't know the answer to is should clicking on a history element should change the currently selected solvers as well. As an example: You have only the dead anomaly type selected and solve problem 1. Then you select the void anomaly type as well and click on the history. Will this deselect void anomaly type again or will it just show no solution for it?

Also would the history be displayed like this or how would it all work? (both would be a problem that both had, void only void, dead only dead)

schweikart · 2024-01-23T08:50:12Z

src/components/solvers/ProgressHandler.tsx

+    // Handle problem states from local storage
+    if (problemStates.length == 0) {
+      let historyItem = localStorage.getItem(historyItemName);
+      if (historyItem == null) return;
+
+      let storageStates = JSON.parse(historyItem);
+      if (storageStates.length > 0) {
+        setProblemStates(storageStates);
+      }
+    }


Can you extract the store / restore code to an external file? useEffect should remain within a component but the code called within may go somewhere else

schweikart · 2024-01-23T08:54:09Z

src/components/solvers/ProgressHandler.tsx

-            <GoButton clicked={startSolving} />
+            <GoButton clicked={getSolution} />


Why did you rename this function? I think the getSolution name hides the fact that this function might invoke an expensive computation of a solution. Also, this getter doesn't return anything

schweikart · 2024-01-23T09:01:39Z

src/components/solvers/ProgressHandler.tsx

+export type ProblemTypeSolutionId = {
+  [problemTypeId: string]: number;
+};


The name is a bit misleading. It seems like an object of this type is an ID. Please give this a clearer name (i.e., SolutionIds) or just use a Map

Elscrux · 2024-01-23T15:24:23Z

@schweikart I've resolved most things you've brought up. The bug in feature model anomaly will need some more decisions before I can go ahead with that. Before we solve this, I won't add a clear history button in case there is a lot of refactoring needed to resolve the bug mentioned before.

sonarqubecloud · 2024-01-23T15:26:39Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Elscrux added 6 commits October 25, 2023 19:44

Remove logical expression parsing

e53a0e9

Remove negated variable token and use separate tokens for negate and …

9341dcd

…variable instead

Add more logical expression validations

60085dc

Refactor text synchronization

5d550c8

Add fetchSolution API call

820b1f5

Add history

6b52dd6

Elscrux requested a review from schweikart November 2, 2023 18:35

Elscrux added 5 commits November 2, 2023 19:37

Give the ListItem key a proper value

a220fbc

Add tooltip to be able to see the full text for long problems

23b0da7

Add load spinner for loading solutions too

4d6fbe0

Add cookie library

59786da

Add cookie to store history

dcc7725

Elscrux added 5 commits November 9, 2023 12:59

Fix using wrong variable for MaxCut text

98bc3c2

Use different cookies for different problem types and useEffect loop

297877a

Don't try to load an existing solution when clicking then Go button

ce58623

Revert "Add cookie library"

7b723a0

This reverts commit 59786da.

Replace cookie approach with local storage

4e254b8

schweikart force-pushed the refactor/remove-logical-expression-parsing branch from 989a5d2 to 8785d74 Compare January 22, 2024 15:23

schweikart force-pushed the feat/history branch from 4e254b8 to f01f3e7 Compare January 22, 2024 16:20

schweikart changed the base branch from refactor/remove-logical-expression-parsing to develop January 22, 2024 16:20

schweikart force-pushed the feat/history branch from f01f3e7 to 4e254b8 Compare January 22, 2024 17:24

schweikart requested changes Jan 23, 2024

View reviewed changes

Elscrux added 3 commits January 23, 2024 13:47

Reduce History api to one function onRequestRollback

1af6a83

Rename content to problemInput

b1d51b2

Rename setText to onTextChanged

a516efc

Elscrux added 3 commits January 23, 2024 16:19

Extract history storage to own file

fb71ba4

Rename getSolution to startSolving

3be04be

Rename ProblemTypeSolutionId to SolutionIds

53e60be

Elscrux added 2 commits January 23, 2024 16:25

Remove code that is commented out

2f5b411

Remove unused import

de8cfb8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem History #39

Problem History #39

Elscrux commented Nov 2, 2023

Elscrux commented Nov 9, 2023 •

edited by schweikart

Loading

sonarqubecloud bot commented Nov 14, 2023

Elscrux commented Nov 14, 2023

schweikart commented Jan 22, 2024 •

edited

Loading

schweikart commented Jan 22, 2024 •

edited

Loading

schweikart left a comment

schweikart Nov 22, 2023

schweikart Nov 22, 2023

Elscrux Jan 23, 2024

schweikart Nov 22, 2023

Elscrux Jan 23, 2024

schweikart Nov 22, 2023

Elscrux Jan 23, 2024

schweikart Nov 22, 2023

Elscrux Jan 23, 2024

schweikart Jan 23, 2024

Elscrux Jan 23, 2024

schweikart Jan 23, 2024

schweikart Jan 23, 2024

Elscrux Jan 23, 2024

Elscrux Jan 23, 2024 •

edited

Loading

schweikart Jan 23, 2024

schweikart Jan 23, 2024

schweikart Jan 23, 2024

Elscrux commented Jan 23, 2024

sonarqubecloud bot commented Jan 23, 2024

		setContent: (t: T) => void;
		loadSolution: (id: ProblemTypeSolutionId) => void;

		<GoButton clicked={startSolving} />
		<GoButton clicked={getSolution} />

Problem History #39

Are you sure you want to change the base?

Problem History #39

Conversation

Elscrux commented Nov 2, 2023

Elscrux commented Nov 9, 2023 • edited by schweikart Loading

sonarqubecloud bot commented Nov 14, 2023

Elscrux commented Nov 14, 2023

schweikart commented Jan 22, 2024 • edited Loading

schweikart commented Jan 22, 2024 • edited Loading

schweikart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elscrux Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elscrux commented Jan 23, 2024

sonarqubecloud bot commented Jan 23, 2024

Quality Gate passed

Elscrux commented Nov 9, 2023 •

edited by schweikart

Loading

schweikart commented Jan 22, 2024 •

edited

Loading

schweikart commented Jan 22, 2024 •

edited

Loading

Elscrux Jan 23, 2024 •

edited

Loading