766 store circuit inputs translation schema #783

ognjenkurtic · 2024-01-22T22:37:34Z

Description

Implements an endpoint through which a circuit input translation schema can be attached to a workstep.
Implements a command handler that validates the provided schema and stores it.
Implements a method in the CircuitInputsParserService for schema validation together with tests
Extends the e2e test to trigger dummy schema creation, in preparation for #777

Related Issue

#766

Motivation and Context

Given in the discussions in the related issues and docs

How Has This Been Tested

Wrote a set of new unit tests
Extended e2e tests
Added an endpoint to Postman collection and tested manually

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Request to be added as a Code Owner/Maintainer

Checklist

My code follows the code style of this project.
[] My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.
I commit to abide by the Responsibilities of Code Owners/Maintainers.

examples/bri-3/prisma/schema.prisma

...ri/workgroup/worksteps/capabilities/setCircuitInputsSchema/setCircuitInputsSchema.command.ts

examples/bri-3/src/bri/workgroup/worksteps/models/workstep.ts

Therecanbeonlyone1969

@ognjenkurtic left some questions. Bit confused.

ognjenkurtic · 2024-01-24T21:14:01Z

@ognjenkurtic left some questions. Bit confused.

@Therecanbeonlyone1969 Please have a look at my answers and let me know if there is anything else to clarify

Therecanbeonlyone1969 · 2024-01-24T22:31:06Z

@ognjenkurtic left some questions. Bit confused.

@Therecanbeonlyone1969 Please have a look at my answers and let me know if there is anything else to clarify

see my replies ... JSON and use a library such as suretype. @ognjenkurtic

skosito

looks good

regarding string vs json, it seems that using json in prisma is straightforward (sorry if assumption is wrong) so maybe lets just do it in this PR

examples/bri-3/prisma/schema.prisma

skosito · 2024-01-29T08:06:37Z

examples/bri-3/src/bri/workgroup/worksteps/agents/worksteps.agent.ts

+  public throwIfCircuitInputTranslationSchemaInvalid(schema): void {
+    const [result, error] =
+      this.cips.validateCircuitInputTranslationSchema(schema);
+    if (!result) {


probably should check if there is error instead of result and returning error imo

renamed 'result' to 'valid' for better readability

skosito · 2024-01-29T08:06:55Z

examples/bri-3/src/bri/workgroup/worksteps/agents/worksteps.agent.ts

+    workstepToUpdate: Workstep,
+    schema: string,
+  ): void {
+    workstepToUpdate.updateCircuitInputTranslationSchema(schema);


can error occur here?

Not really, asigning string value

skosito · 2024-01-29T08:08:34Z

examples/bri-3/src/bri/workgroup/worksteps/api/worksteps.controller.ts

@@ -71,6 +73,17 @@ export class WorkstepController {
    );
  }

+  @Put('/:id/circuitinputsschema')
+  @CheckAuthz({ action: 'update', type: 'Workstep' })
+  async setCircuitInputsSchemaCommand(


small thing but i would name this update instead of set to be clear, but up to you

skosito · 2024-01-29T08:13:54Z

...rc/bri/zeroKnowledgeProof/services/circuit/circuitInputsParser/circuitInputParser.service.ts

+        }
+      }
+
+      return [true, ''];


i dont think we should return both boolean and error, looks kinda weird in javascript, lets just return error which implies that validation is true or false? but up to you

@ognjenkurtic are you going to leave it like that? L57 still returns a boolean and error message. Same with L55 ... I would return a No-Error Message

CLAassistant · 2024-04-08T09:49:36Z

All committers have signed the CLA.

Therecanbeonlyone1969

Had one comment as a follow-up. Once addressed, good to be merged after resolving merge conflicts.

ognjenkurtic · 2024-06-19T20:17:21Z

@skosito @Therecanbeonlyone1969 waiting for the other one to be merged to main so that i can resolve confilcts here and verify the e2e test. Everything else is addressed.

…ircuitInputParser service

…d to postman collection; Rename add cis command to set cis command;

…ch of tests

…hrow error in case validation fails

…ter redability

…string representing error

ognjenkurtic · 2024-06-20T14:38:31Z

@Therecanbeonlyone1969 @biscuitdey @skosito ready for rereview. tests green, rebased on top of main.

Therecanbeonlyone1969

LGTM

biscuitdey

Looks good!

ognjenkurtic requested review from skarred14, Therecanbeonlyone1969, Kasshern, Ybittan, biscuitdey and skosito as code owners January 22, 2024 22:37

ognjenkurtic changed the base branch from main to feat/776-circuit-inputs-parser January 22, 2024 22:38

Therecanbeonlyone1969 reviewed Jan 24, 2024

View reviewed changes

examples/bri-3/prisma/schema.prisma Outdated Show resolved Hide resolved

Therecanbeonlyone1969 reviewed Jan 24, 2024

View reviewed changes

...ri/workgroup/worksteps/capabilities/setCircuitInputsSchema/setCircuitInputsSchema.command.ts Outdated Show resolved Hide resolved

Therecanbeonlyone1969 reviewed Jan 24, 2024

View reviewed changes

examples/bri-3/src/bri/workgroup/worksteps/models/workstep.ts Show resolved Hide resolved

Therecanbeonlyone1969 reviewed Jan 24, 2024

View reviewed changes

skosito reviewed Jan 29, 2024

View reviewed changes

Base automatically changed from feat/776-circuit-inputs-parser to main February 5, 2024 21:44

ognjenkurtic requested review from skosito and Therecanbeonlyone1969 April 12, 2024 21:17

Therecanbeonlyone1969 previously approved these changes Apr 15, 2024

View reviewed changes

ognjenkurtic dismissed Therecanbeonlyone1969’s stale review via 9295224 June 19, 2024 20:15

ognjenkurtic added 9 commits June 20, 2024 15:27

Tell vscode to use workspace typescript version

8def4ff

Add circuitInputsTranslationSchema field to workstep + migration

7386e11

Add extremely basic validateCircuitInputTranslationSchema method to c…

ffaa8f9

…ircuitInputParser service

Implement first version of AddCircuitInputsSchemaCommand with a handler

6699c25

Implement endpoint for setting circuit input schema on a workstep; ad…

52dcf72

…d to postman collection; Rename add cis command to set cis command;

Run prettier

cfb5f85

Extend validateCircuitInputTranslationSchema method and add first bat…

b577957

…ch of tests

Add dummy circuit input schema creation step to e2e test

a0c5287

validateCircuitInputTranslationSchema - Parse validation reuslt and t…

0d90443

…hrow error in case validation fails

ognjenkurtic added 8 commits June 20, 2024 15:28

Rename output param for validateCircuitInputTranslationSchema for bet…

999feda

…ter redability

Rename setCircuitInputsSchema command to updateCircuitInputsSchema

fc2b672

Add circuitInputsTranslationSchema as JSON column to workstep

f869d30

Update postman collection with example schema

410ab21

Finish rename of SetCircuitInputSchema to UpdateCircuitInputSchema

6c30aa3

Cleanup from local testing

05ee92f

Fix validateCircuitInputTranslationSchema return type to be a single …

af581fc

…string representing error

Fix failing unit tests related to circuit input parsers

3c9230d

ognjenkurtic force-pushed the 766-store-circuit-inputs-translation-schema branch from 9295224 to 3c9230d Compare June 20, 2024 14:03

Fix assert for invalid JSON schema test in ciruit parser service

0dc2cf3

Therecanbeonlyone1969 approved these changes Jun 20, 2024

View reviewed changes

biscuitdey approved these changes Jun 25, 2024

View reviewed changes

biscuitdey merged commit 1f57a18 into main Jun 25, 2024
2 checks passed

biscuitdey deleted the 766-store-circuit-inputs-translation-schema branch June 25, 2024 08:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

766 store circuit inputs translation schema #783

766 store circuit inputs translation schema #783

ognjenkurtic commented Jan 22, 2024

Therecanbeonlyone1969 left a comment

ognjenkurtic commented Jan 24, 2024

Therecanbeonlyone1969 commented Jan 24, 2024

skosito left a comment •

edited

Loading

skosito Jan 29, 2024

ognjenkurtic Apr 11, 2024

skosito Jan 29, 2024

ognjenkurtic Apr 11, 2024

skosito Jan 29, 2024

ognjenkurtic Apr 11, 2024

skosito Jan 29, 2024

ognjenkurtic Apr 11, 2024

Therecanbeonlyone1969 Apr 15, 2024

ognjenkurtic Jun 19, 2024

CLAassistant commented Apr 8, 2024 •

edited

Loading

Therecanbeonlyone1969 left a comment •

edited

Loading

ognjenkurtic commented Jun 19, 2024

ognjenkurtic commented Jun 20, 2024

Therecanbeonlyone1969 left a comment

biscuitdey left a comment

766 store circuit inputs translation schema #783

766 store circuit inputs translation schema #783

Conversation

ognjenkurtic commented Jan 22, 2024

Description

Related Issue

Motivation and Context

How Has This Been Tested

Screenshots (if appropriate)

Types of changes

Checklist

Therecanbeonlyone1969 left a comment

Choose a reason for hiding this comment

ognjenkurtic commented Jan 24, 2024

Therecanbeonlyone1969 commented Jan 24, 2024

skosito left a comment • 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

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

CLAassistant commented Apr 8, 2024 • edited Loading

Therecanbeonlyone1969 left a comment • edited Loading

Choose a reason for hiding this comment

ognjenkurtic commented Jun 19, 2024

ognjenkurtic commented Jun 20, 2024

Therecanbeonlyone1969 left a comment

Choose a reason for hiding this comment

biscuitdey left a comment

Choose a reason for hiding this comment

skosito left a comment •

edited

Loading

CLAassistant commented Apr 8, 2024 •

edited

Loading

Therecanbeonlyone1969 left a comment •

edited

Loading