Skip to content
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

Prettify metadata when fetching assets and devices from API #228

Merged
merged 25 commits into from
Oct 18, 2022
Merged

Conversation

TisserantG
Copy link
Contributor

No description provided.

Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

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

You should have starts from the 2-dev branch because it had the new module architecture

Comment on lines +209 to +239
(request.input.args.collection === "assets" ||
request.input.args.collection === "devices") &&
request.input.args.options?.prettify
) {
for (const document of documents) {
if (document._source?.metadata) {
document._source.metadata =
this.assetCategoryService.formatMetadataForGet(
document._source.metadata
);
}
}
}
return documents;
},
"generic:document:beforeUpdate": async (documents, request: Request) => {
if (
(request.input.args.collection === "assets" ||
request.input.args.collection === "devices") &&
request.input.args.options?.prettify
) {
for (const document of documents) {
if (document._source?.metadata) {
document._source.metadata =
this.assetCategoryService.formatMetadataForES(
document._source.metadata
);
}
}
}
return documents;
Copy link
Contributor

Choose a reason for hiding this comment

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

This part could be DRY'ied

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do something like

pipeFunction(myFunction)
        if (
          (request.input.args.collection === "assets" ||
            request.input.args.collection === "devices") &&
          request.input.args.options?.prettify
        ) {
          for (const document of documents) {
            if (document._source?.metadata) {
              document._source.metadata =
                myFunction(
                  document._source.metadata
                );
            }
          }
        }
...
this.pipes = {
     ....
      "generic:document:afterGet": async (documents, request: Request) => {
       return pipeFunction( this.assetCategoryService.formatMetadataForGet);}
      "generic:document:beforeUpdate": async (documents, request: Request) => {
       return pipeFunction(this.assetCategoryService.formatMetadataForES);}
};

It's more concise, but, in my opinion, (even with better naming) less readable.

lib/models/Device.ts Outdated Show resolved Hide resolved
lib/core-classes/AssetCategoryService.ts Outdated Show resolved Hide resolved
lib/types/DeviceContent.ts Outdated Show resolved Hide resolved
@TisserantG
Copy link
Contributor Author

You should have starts from the 2-dev branch because it had the new module architecture

Yes, but when I start to work, the new architecture was not merged :-(

@Aschen Aschen changed the base branch from 2-stable to 2-dev October 11, 2022 15:37
@Aschen Aschen changed the title Feat/optin Prettify metadata when fetching assets and devices from API Oct 11, 2022
@Aschen
Copy link
Contributor

Aschen commented Oct 11, 2022

You should have starts from the 2-dev branch because it had the new module architecture

Yes, but when I start to work, the new architecture was not merged :-(

I checked and it was not that hard, I did it ;-)

Aschen and others added 2 commits October 11, 2022 17:39
@Aschen Aschen merged commit d0160bb into 2-dev Oct 18, 2022
@Aschen Aschen deleted the feat/optin branch October 18, 2022 07:01
@Aschen Aschen mentioned this pull request Oct 18, 2022
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.

None yet

3 participants