feat: implement renderer 2024 provider #177

ducpm511 · 2024-05-07T06:32:55Z

What type of PR is this? (check all applicable)

Description

This PR is about implementing a new renderer provider and enhancing the UI of demo explorer to display the new templates.

Related Tickets & Documents

https://github.com/gs-gs/fa-ag-trace/issues/430
https://github.com/gs-gs/fa-ag-trace/issues/431

Mobile & Desktop Screenshots/Recordings

new-renderer.mov

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help

Added to documentation?

📜 README.md
📓 vc-kit doc site
📕 storybook
🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

# Conflicts: # packages/cli/default/default-dev.yml # packages/cli/default/default.yml # packages/core-types/package.json # packages/core-types/src/index.ts # packages/core-types/src/plugin.schema.json

namhoang1604 · 2024-05-09T02:57:18Z

packages/renderer/src/providers/web-rendering-template-2024.ts

+      if (
+        digestMultibase &&
+        context &&
+        typeof context.agent.createVerifiableCredentialOA === 'function'


Why do we need to check createVerifiableCredentialOA?

namhoang1604 · 2024-05-09T02:58:37Z

packages/renderer/src/providers/web-rendering-template-2024.ts

+/**
+ * @public
+ */
+export class WebRenderingTemplate2024 implements IRendererProvider {


Based on w3c-ccg/vc-render-method#9, the name should be RenderTemplate2024

namhoang1604 · 2024-05-09T03:00:56Z

packages/renderer/src/providers/web-rendering-template-2024.ts

+    context?: IRendererContext;
+  }): Promise<string> {
+    try {
+      let svgTemplate: string | any = '';


Why the name of the variable is svgTemplate ? And the type should be only string

namhoang1604 · 2024-05-09T03:20:34Z

packages/renderer/src/providers/web-rendering-template-2024.ts

+          return('<p style="color: red">Error: Template hash does not match the provided digest</p>');
+        }
+      }
+      const compiledTemplate = handlebars.compile(svgTemplate);


Should we check the mediaType to choose the template engine? Like the XML can not use with the handlebars

namhoang1604 · 2024-05-09T03:21:48Z

packages/renderer/src/renderer.ts

 } from '@vckit/core-types';
 import schema from '@vckit/core-types/build/plugin.schema.json' assert { type: 'json' };
+import { url } from 'inspector';


where is it used?

namhoang1604 · 2024-05-09T03:23:43Z

packages/renderer/src/renderer.ts

@@ -82,30 +83,55 @@ export class Renderer implements IAgentPlugin {

      // TODO: There's an issue with W3 availability causing the fetching of some W3 context files to fail. Since we know the exact location of the template we can bypass the JSONLD expansion. This is a temporary workaround.

-      const render = args.credential.render;
+      const render = args.credential.render || args.credential.renderMethod;


Previous, we had hotfix for the performance issue. So please follow the TODO to handle it right

namhoang1604 · 2024-05-09T03:29:55Z

packages/renderer/src/renderer.ts

-          return convertToBase64(document);
+          const responseDocument = {
+            type: renderMethod['@type'],
+            renderedTemplate: convertToBase64(document),
+            id: renderMethod.id,
+            name: renderMethod.name,
+            mediaType: renderMethod.mediaType,
+
+          };
+          return responseDocument;


Is the new output backward compatible with the old WebRenderingTemplate2022?

# Conflicts: # pnpm-lock.yaml

* next: chore: fix version of react feat: implement vc api v2 in open api (#178) # Conflicts: # pnpm-lock.yaml

* next: chore: fix version of react and react-dom

* next: fix: add interface for compute hash (#179)

* next: chore: fix the dockerfile to set entrypoint chore: update vckit Dockerfile (#184) chore: update DockerFile and agent template (#183) fix: bug fix for dockerize (#174) chore: add cicd for running unit test when creating PR (#180) chore: update agent config file (#181) # Conflicts: # pnpm-lock.yaml

* next: chore: change node version to 20.12.2 (#187) chore: update default agent for Status plugin (#186)

* next: docs: add yarn to prerequisites

* next: fix: using revocation list on explorer and handle no hash (#192)

github-actions · 2024-06-10T07:43:19Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟢	Statements	97.55%	398/408
🟢	Branches	83.08%	54/65
🟢	Functions	92.86%	13/14
🟢	Lines	97.55%	398/408

Test suite run success

21 tests passing in 3 suites.

Report generated by 🧪jest coverage report action from 25ff963

packages/renderer/__tests__/providers/render-template-2024.test.ts

namhoang1604 · 2024-06-12T08:11:19Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+    expect(result.renderedTemplate).toBe('Error: Unsupported media type');
+  });
+
+  it('should return empty string if it failed to fetch template from url', async () => {


is this test case duplicated https://github.com/uncefact/project-vckit/pull/177/files#diff-814e4aee83e82ae7cb7da30924b63f20d9188fa64e1049e70127a80301b9b584R279-R299

no it's not. this is to cover the statement in the code(like if-else, try-catch etc...)

namhoang1604 · 2024-06-12T08:13:16Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+    expect(result.renderedTemplate).toBe('');
+  });
+
+  it('should return template with style added', async () => {


is this test case duplicated https://github.com/uncefact/project-vckit/pull/177/files#diff-814e4aee83e82ae7cb7da30924b63f20d9188fa64e1049e70127a80301b9b584R210-R243

this is to reduce the uncovered count for statements as well, so yes, we need this case

Disagree, 2 test cases are mostly the same, please review it again

namhoang1604 · 2024-06-12T08:51:53Z

packages/renderer/src/providers/render-template-2024.ts

+      if (url) {
+        try {
+          const response = await fetch(url);
+          renderTemplate = await response.text();
+        } catch (error) {
+          console.error(`Failed to fetch from ${url}:`, error);
+        }
+      }else if (template) {
+        renderTemplate = template;
+      }else{
+        return {
+          renderedTemplate: 'Error: No template or url provided',
+        };
+      }


How does the condition expression cover the case of the fetching URL is timeout, then using inline template to render?

thanks @namhoang1604 I've handled this

… the case that fetch template from url

huynguyen-hl · 2024-06-12T11:19:55Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+    expect(result).toStrictEqual(
+      {"renderedTemplate": "Error: No hash function provided to verify the template"}
+    );


I think we should assert with the error message Error: Template hash does not match the provided digest for this test case. Because the title suggests comparing the template hashed string with the digestMultibase value.

namhoang1604 · 2024-06-13T01:41:44Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+    await expect(async () => {
+      await renderer.renderCredential({
+        data,
+        document,
+      });
+    }).rejects.toThrow();


The test case is should return an error message if the template is not a string, but the expect is throw error

namhoang1604 · 2024-06-13T01:50:35Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+  it('should return an error message if the template is not provided and there is no default template', async () => {
+    const data = {
+      'https://w3id.org/security#digestMultibase': [
+        {
+          '@type': 'https://w3id.org/security#multibase',
+          '@value': '',
+        },
+      ],
+      'https://www.w3.org/2018/credentials#renderMethod#mediaQuery': [
+        {
+          '@value':
+            '',
+        },
+      ],
+      'https://schema.org/encodingFormat': [
+        {
+          '@value': 'text/html',
+        },
+      ],
+      'https://www.w3.org/2018/credentials#renderMethod#url': [
+        {
+          '@value': '',
+        },
+      ],
+    };
+    const rendererWithoutDefaultTemplate = new RenderTemplate2024();
+    const result = await rendererWithoutDefaultTemplate.renderCredential({ data, document: {} });
+    expect(result).toStrictEqual(
+      {"renderedTemplate": "Error: No template or url provided"}
+    );
+  });


What is the default template? Please correct the test case

namhoang1604 · 2024-06-13T01:55:08Z

packages/renderer/__tests__/providers/render-template-2024.test.ts

+    expect(result.renderedTemplate).toBe('');
+  });
+
+  it('should return template with style added', async () => {


Disagree, 2 test cases are mostly the same, please review it again

namhoang1604 · 2024-06-13T02:04:37Z

packages/renderer/src/providers/web-rendering-template-2022.ts

+    const template = this.extractTemplate(data);
+    if (!template?.trim()) {
+      return {
+        renderedTemplate: 'Error: invalid template provided',
+      };
    }


Why is the empty template invalid? Where is the document saying that?

namhoang1604 · 2024-06-13T02:05:01Z

packages/renderer/src/providers/web-rendering-template-2022.ts

+  }
+
+  private extractTemplate(data: any) {
+    // const RENDER_METHOD = 'https://www.w3.org/2018/credentials#renderMethod';


remove the comment

namhoang1604 · 2024-06-13T02:08:56Z

packages/renderer/__tests__/providers/web-rendering-template-2022.test.ts

+    });
+
+    it('should throw an error if the template is not a valid handlebars template', async () => {
+      // const template = '<p>{{name}}</p>';


Remove comment

namhoang1604 · 2024-06-13T02:13:13Z

packages/renderer/__tests__/renderer.test.ts

-      'PGRpdiBzdHlsZT0id2lkdGg6MzAwcHg7IGhlaWdodDoxMDBweDsgYm9yZGVyOiAycHggc29saWQgYmxhY2s7IHRleHQtYWxpZ246Y2VudGVyIj4KICA8ZGl2PgogICAgVGhpcyBCYWNoZWxvciBvZiBTY2llbmNlIGFuZCBBcnRzIGlzIGNvbmZlcnJlZCB0bwogIDwvZGl2PgogIDxzdHJvbmcgc3R5bGU9ImZvbnQtc2l6ZTogMTZweCI+CiAgICBKYW5lIFNtaXRoCiAgPC9zdHJvbmc+CiAgPGRpdj4KICAgIGJ5IEV4YW1wbGUgVW5pdmVyc2l0eS4KICA8L2Rpdj4KPC9kaXY+',
-    ]);
+    expect(result.documents).toEqual([{
+      "renderedTemplate": "RXJyb3I6IGludmFsaWQgdGVtcGxhdGUgcHJvdmlkZWQ=",


Why the result of renderedTemplate is Error: invalid template provided? That should render the template successfully

namhoang1604 · 2024-06-13T02:17:34Z

packages/renderer/__tests__/renderer.test.ts

@@ -115,29 +140,26 @@ describe('Renderer', () => {
    ).rejects.toThrow('Render method not found in the verifiable credential');
  });

-  it('should throw an error if the verifiable credential contains an invalid render method', async () => {
+  it('should throw an error with invalid @type and non-default provider', async () => {


Why do you edit the important test case?

namhoang1604 · 2024-06-13T02:19:57Z

packages/renderer/__tests__/renderer.test.ts

  });

-  it('should skip render methods without @type or template', async () => {


Please do not remove the required test cases

namhoang1604 · 2024-06-13T02:21:13Z

packages/renderer/__tests__/renderer.test.ts

-  it('should throw an error with invalid @type and non-default provider', async () => {
+  
+  it('should throw an error if there is no context file for renderer', async () => {


Please do not remove the required test cases, you can add new ones

Signed-off-by: Nam Hoang <[email protected]>

ldhyen99 and others added 11 commits April 27, 2024 00:06

feat: add compute hash utils

61ed51f

feat: add prefix in compute hash function

034f537

feat: create plugin that contain hash

ca1e3f4

test: add unit test for compute hash

14fc4b8

feat: implement renderer 2024 provider

a49ee9c

feat: add plugin to agent.yml

f02873d

refactor: delete compute hash function in tools package

b572e1d

chore: update ts config file

fdac6c1

Merge branch 'feat/add_compute_hash_utils' into renderer2024

ec3746f

feat: implement new UI for renderer on demo explorer

9a033de

Merge branch 'next' into renderer2024

713e011

# Conflicts: # packages/cli/default/default-dev.yml # packages/cli/default/default.yml # packages/core-types/package.json # packages/core-types/src/index.ts # packages/core-types/src/plugin.schema.json

ducpm511 requested review from namhoang1604, huynguyen-hl and ldhyen99 May 7, 2024 06:32

ducpm511 added 2 commits May 7, 2024 14:26

feat: updated unit tests

5afd5ab

feat: handled error

caef6c5

huynguyen-hl previously approved these changes May 8, 2024

View reviewed changes

namhoang1604 requested changes May 9, 2024

View reviewed changes

huynguyen-hl self-requested a review May 9, 2024 14:50

ducpm511 and others added 7 commits May 14, 2024 08:40

Merge branch 'next' into renderer2024

4001ef1

# Conflicts: # pnpm-lock.yaml

feat: updated according to feedback

d3581cc

feat: updated renderer to be able to use extractRendererMethod function

2f05cca

refactor: clean up renderer

85b33a0

Merge branch 'next' into renderer2024

7b4ac12

* next: chore: fix version of react feat: implement vc api v2 in open api (#178) # Conflicts: # pnpm-lock.yaml

refactor: adding missing template

65b9a24

Merge branch 'next' into renderer2024

c55545e

* next: chore: fix version of react and react-dom

ducpm511 changed the title ~~Renderer2024~~ feat: implement renderer 2024 provider May 17, 2024

ducpm511 added 3 commits May 17, 2024 15:57

refactor: renamed provider name; replace context url

fec182d

Merge branch 'next' into renderer2024

8891f63

* next: fix: add interface for compute hash (#179)

refactor: updated provider according to computeHash function changes

edcfcc0

ducpm511 dismissed namhoang1604’s stale review via e035b72 June 4, 2024 08:33

namhoang1604 previously approved these changes Jun 4, 2024

View reviewed changes

chore: updated unit test corresponding to changes of render provider

ab74283

ducpm511 dismissed namhoang1604’s stale review via ab74283 June 6, 2024 02:34

ducpm511 added 7 commits June 6, 2024 09:47

chore: fix renderer to address unit test

d3bcc29

Merge branch 'next' into renderer2024

2080de3

* next: chore: change node version to 20.12.2 (#187) chore: update default agent for Status plugin (#186)

Merge branch 'next' into renderer2024

ebca386

* next: docs: add yarn to prerequisites

chore: added more test case to increase the coverage

cce6eb6

chore: added more unit test cases

1d602ad

Merge branch 'next' into renderer2024

5fbca74

* next: fix: using revocation list on explorer and handle no hash (#192)

chore: reduced uncovered count for statement

46f7bcf

Merge branch 'next' into renderer2024

c370dea

namhoang1604 requested changes Jun 12, 2024

View reviewed changes

chore: removed duplicate test case

a65aae1

namhoang1604 reviewed Jun 12, 2024

View reviewed changes

refactor: use the inline template if failed to fetch it from url.

d4e31b1

ducpm511 requested a review from namhoang1604 June 12, 2024 09:15

fix: removed duplicate test case and implement mock fetch to simulate…

75c4d42

… the case that fetch template from url

huynguyen-hl reviewed Jun 12, 2024

View reviewed changes

namhoang1604 requested changes Jun 13, 2024

View reviewed changes

ducpm511 and others added 4 commits June 13, 2024 09:41

fix: wrong expected result

aa46d60

chore: removed meanless comment

50c45c4

refactor: enhance the render template 2024

ed4ec10

Signed-off-by: Nam Hoang <[email protected]>

test: adjust to update the unit test of renderer plugin

25ff963

Signed-off-by: Nam Hoang <[email protected]>

namhoang1604 force-pushed the renderer2024 branch from f9901ad to 25ff963 Compare June 13, 2024 09:30

namhoang1604 approved these changes Jun 13, 2024

View reviewed changes

namhoang1604 merged commit 7bcbde3 into next Jun 13, 2024
3 checks passed

namhoang1604 deleted the renderer2024 branch June 13, 2024 09:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement renderer 2024 provider #177

feat: implement renderer 2024 provider #177

ducpm511 commented May 7, 2024 •

edited

Loading

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

namhoang1604 May 9, 2024

github-actions bot commented Jun 10, 2024 •

edited

Loading

namhoang1604 Jun 12, 2024

ducpm511 Jun 12, 2024

namhoang1604 Jun 12, 2024

ducpm511 Jun 12, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 12, 2024

ducpm511 Jun 12, 2024

huynguyen-hl Jun 12, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

namhoang1604 Jun 13, 2024

		});

		it('should skip render methods without @type or template', async () => {

		it('should throw an error with invalid @type and non-default provider', async () => {

		it('should throw an error if there is no context file for renderer', async () => {

feat: implement renderer 2024 provider #177

feat: implement renderer 2024 provider #177

Conversation

ducpm511 commented May 7, 2024 • edited Loading

What type of PR is this? (check all applicable)

Description

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

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

github-actions bot commented Jun 10, 2024 • edited Loading

Coverage report

Test suite run success

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ducpm511 commented May 7, 2024 •

edited

Loading

github-actions bot commented Jun 10, 2024 •

edited

Loading