Skip to content

Handle Updated Claims #389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 48 commits into from
Nov 14, 2018
Merged

Handle Updated Claims #389

merged 48 commits into from
Nov 14, 2018

Conversation

jwicks31
Copy link
Contributor

@jwicks31 jwicks31 commented Nov 6, 2018

PR Process - PR Review Checklist

Description of Changes

Resolves #365 and Resolves #393 (If we are able to use it)

  • Updated work attributes to claim
  • Added Interfaces from updated poet-js
    • I copied them over for now because there is a slight change I made to Claim by defining an interface rather than using the object type. This was so that I could read the individual claim properties.
    • Another problem with importing the interfaces from poet-js is that it breaks the build because we do not have the correct loaded for something with jsonld

I wasn't sure how 'Dynamic' the claims needed to be.

Above the Work/Claim it will show an author, datePublished, dateModified, name, tags if they are available. If there is no author, it labels it "Unknown Author". If there is no name, it labels it "Untitled Work". It will not show anything for datePublished, dateModified, or tags if they aren't supplied.

In the Content Section of the Work/Claim it will show any keys/values that are supplied and not empty. It filters out for "text" and "content" and displays whichever of those is supplied below the other key/value pairs.

In the Technical Tab it lists all key/values form the timestamp attribute.

An example work id to test that the claims are loading is : a9cef68009c16fbaafabc2d423c269311a9392f2ce5a51f4944711bdfa73f1b7

The Netlify Deploy preview will open to regtest, but the work ID above is in testnet. Change the regtest in the link to say testnet eg:
https://deploy-preview-389--explorer-web-qa-testnet.netlify.com/works/a9cef68009c16fbaafabc2d423c269311a9392f2ce5a51f4944711bdfa73f1b7

screen shot 2018-11-13 at 12 30 05 pm

screen shot 2018-11-13 at 12 29 42 pm

screen shot 2018-11-13 at 12 29 49 pm

@jwicks31 jwicks31 self-assigned this Nov 6, 2018
@lautarodragan
Copy link
Member

@jwicks31 @krobi64 @geoffturk what's the scope of this task? Should explorer-web be smart enough to parse the @context and dynamically refer to attributes based on it?

@krobi64
Copy link

krobi64 commented Nov 8, 2018

I am not sure that is required for mainnet. However, I do believe that we should look that way long term. jsonld can render the json and context with the appropriate schema references as the attribute name. For example:

{
   "schema:Book": {
     "schema:author": {
         "schema:name": {
             "value": "Sir Author Conan Doyle"
         }
     },
    "schema:title": "The Hound of the Baskervilles"
   }
}

This allows us to create a more flexible interface.

@jwicks31
Copy link
Contributor Author

jwicks31 commented Nov 8, 2018

I wasn't really sure of the scope either. I do think the scope for this PR should just be getting it functioning the way it was before, but with the new claim types. Then if we want to handle that change doing it separately.

@lautarodragan
Copy link
Member

Thanks @krobi64

@jwicks31 agreed.

@@ -125,13 +125,18 @@ module.exports = {
resolve: {
extensions: ['.webpack.js', '.web.js', '.ts', '.tsx', '.js', '.json', '.css', '.scss'],
alias: {
Configuration: path.resolve(configurationPath)
Configuration: path.resolve(configurationPath),
jsonld$: path.resolve(__dirname, './node_modules/jsonld/dist/jsonld.min.js')
Copy link
Contributor Author

@jwicks31 jwicks31 Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lautarodragan @krobi64 When using @po.et/poet-js package I'm running into several problems. First the json-ld package was causing the error:

ERROR in ./node_modules/jsonld/lib/jsonld.js
Module parse failed: Unexpected token (1040:2)
You may need an appropriate loader to handle this file type.
| function _setDefaults(options, {
|   documentLoader = jsonld.documentLoader,
|   ...defaults
| }) {
|   if(typeof options === 'function') {
 @ ./src/utils/JSONLD.ts 3:15-32
 @ ./src/index.ts
Warning:  Use --force to continue.

Aborted due to warnings.

I was able to fix this in my local build with this line, which I got from the issues page: digitalbazaar/jsonld.js#252 (comment)

},
modules: [
path.join(__dirname, "src"),
"node_modules"
],
},

node: {
net: 'empty',
Copy link
Contributor Author

@jwicks31 jwicks31 Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lautarodragan @krobi64 The joi dependency in poet-js was causing this error:
Module not found: Error: Cannot resolve module 'net' on node_modules/joi/lib @ ./~/joi/lib/string.js 3:10-24
I was able to fix this locally with this line, which I got from the joi issues: hapijs/joi#665 (comment)

Copy link
Contributor

@wzalazar wzalazar Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhmm because joi is not prepared to use in the front end. For this reason, I saw in several projects are using https://github.com/jquense/yup instead of joi

Maybe if we want to use poet-js in the backend and frontend, we have to use a library that works fine on the two sides

@jwicks31
Copy link
Contributor Author

@krobi64 @lautarodragan After making those changes to get everything building locally I was still getting a long list of errors in the Netlify Build that starts with:

> node-gyp rebuild
12:06:09 PM: make: Entering directory `/opt/build/repo/node_modules/rdf-canonize/build'
12:06:09 PM:   CXX(target) Release/obj.target/urdna2015/lib/native/IdentifierIssuer.o
12:06:10 PM:   CXX(target) Release/obj.target/urdna2015/lib/native/MessageDigest.o
12:06:10 PM:   CXX(target) Release/obj.target/urdna2015/lib/native/NQuads.o
12:06:10 PM: ../lib/native/NQuads.cc: In static member function ‘static std::string RdfCanonize::NQuads::serializeQuad(const RdfCanonize::Quad&)’:
12:06:10 PM: ../lib/native/NQuads.cc:72:62: error: no matching function for call to ‘regex_replace(std::string&, std::regex&, const char [3])’
12:06:11 PM:      escaped = regex_replace(escaped, REGEX_BACK_SLASH, "\\\\");
12:06:11 PM:                                                               ^
12:06:11 PM: ../lib/native/NQuads.cc:72:62: note: candidates are:
12:06:11 PM: In file included from /usr/include/c++/4.8/regex:62:0,
12:06:11 PM:                  from ../lib/native/NQuads.cc:13:
12:06:11 PM: /usr/include/c++/4.8/bits/regex.h:2162:5: note: template<class _Out_iter, class _Bi_iter, class _Rx_traits, class _Ch_type> _Out_iter std::regex_replace(_Out_iter, _Bi_iter, _Bi_iter, const std::basic_regex<_Ch_type, _Rx_traits>&, const std::basic_string<_Ch_type>&, std::regex_constants::match_flag_type)
12:06:11 PM:      regex_replace(_Out_iter __out, _Bi_iter __first, _Bi_iter __last,
12:06:11 PM:      ^
12:06:11 PM: /usr/include/c++/4.8/bits/regex.h:2162:5: note:   template argument deduction/substitution failed:
12:06:11 PM: ../lib/native/NQuads.cc:72:62: note:   deduced conflicting types for parameter ‘_Bi_iter’ (‘std::basic_regex<char>’ and ‘const char*’)

Uninstalling the package and just copying the interfaces over solved the issue, but other than that I haven't been able to find a solution. Another issue is that after updating it to the newest version security/snyk started to fail with 3 errors, but I was unable to read what they were. @warrenv

@krobi64
Copy link

krobi64 commented Nov 13, 2018

@jwicks31 For the build error, use this in the Dockerfile:

RUN apt-get update && apt-get install -y rsync \
                       gcc-5 \
                       g++-5 \
                       && rm -rf /var/lib/apt/lists/* \
                       && rm /etc/apt/sources.list.d/unstable.list

@wzalazar
Copy link
Contributor

@jwicks31 @krobi64 we had this issue before and we can't install dependencies on Netlify

@jwicks31
Copy link
Contributor Author

jwicks31 commented Nov 13, 2018

@krobi64 @wzalazar Yeah I remember having this issue with Frost-web as well. I think we were trying to modify the Netlify.toml file though, so I went ahead with updating Docker to try. Still failing with the same errors though.

@krobi64
Copy link

krobi64 commented Nov 13, 2018

@jwicks31 @wzalazar that seems to be a serious limitation on Netlifty. Can we build and push a docker image that Netlifty then downloads?

@jwicks31
Copy link
Contributor Author

@krobi64 I'm not really sure, but I'm not finding anything on how to do it.

@krobi64
Copy link

krobi64 commented Nov 14, 2018

Ok, for now, let's remove po.et and bring the interface. It may be that the frost api will have to eventually return the object already deconstructed... but that's out of scope

@jwicks31
Copy link
Contributor Author

@wzalazar @lautarodragan @krobi64 This one is ready for review now. Thank you!

Copy link
Member

@lautarodragan lautarodragan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jwicks31 jwicks31 merged commit 910d8ca into master Nov 14, 2018
@jwicks31 jwicks31 deleted the claims branch November 14, 2018 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to latest poet-js package Handle claim properties dynamically
4 participants