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

[OC-102] Return rendered version if client doesn't support #605

Merged
merged 4 commits into from
Aug 17, 2017

Conversation

nickbalestra
Copy link
Contributor

Related to (and blocked by)

Registry will handle server side rendering in case of oc-client requesting the unrendered version of an unsupported template

Copy link
Member

@mattiaerre mattiaerre left a comment

Choose a reason for hiding this comment

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

over to you @nickbalestra

@@ -145,6 +145,15 @@ module.exports = function(conf, repository) {
});
}

// Support legacy templates
let templateType = component.oc.files.template.type;
Copy link
Member

Choose a reason for hiding this comment

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

at this point, we are positive that component.oc.files.template is not undefined right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@@ -96,4 +96,72 @@ describe('registry : routes : helpers : get-component', () => {
expect(fireStub.args[0][1].status).to.equal(500);
});
});

describe("when oc-client request an unrendered component and it doesn't support the correct template", () => {
Copy link
Member

Choose a reason for hiding this comment

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

very minor but I would still use single quotes and either escape the apostrophe w/ a '' or even consider to use '`' instead.

also the describe should be when oc-client requests ...

@nickbalestra
Copy link
Contributor Author

@mattiaerre fixed

Copy link
Member

@mattiaerre mattiaerre left a comment

Choose a reason for hiding this comment

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

this LGTM shall I merge or wait until is [Unblocked]?

@nickbalestra
Copy link
Contributor Author

Yes lets wait till we publish the client first as we need to update the client dependency in OC too for this to work correctly

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.

2 participants