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

Remove File Type Mismatch Workaround #158

Closed
rgauss opened this issue Aug 25, 2021 · 4 comments
Closed

Remove File Type Mismatch Workaround #158

rgauss opened this issue Aug 25, 2021 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@rgauss
Copy link
Contributor

rgauss commented Aug 25, 2021

See:

We should remove the workarounds when those issues have been fixed.

@laurelmay
Copy link
Contributor

It looks like the upstream issue has been closed (as the underlying bug has been resolved) but the updated version of the submodule will not be available until usnistgov/oscal-content tags a v1.0.1 release.

@laurelmay laurelmay added good first issue Good for newcomers chore Cleanup or repository management work labels Aug 15, 2022
@laurelmay
Copy link
Contributor

This has been fixed on the main branch of the project and therefore I am going assign this to a milestone so we can track the work on it. We don't currently reference tagged released upstream anyway and staring at the now mostly dead code for fixJsonUrls just makes me a little sad.

For reference, the workaround to remove is

export function fixJsonUrls(absoluteUrl) {
// TODO: This workaround references JSON representation instead of the back-matter links to
// XML catalogs and it should be removed once the issue has been resolved.
// https://github.com/EasyDynamics/oscal-react-library/issues/158
if (!absoluteUrl.endsWith(".xml")) {
return absoluteUrl;
}
// Replacing all instances of xml with json in the path *should* get us the correct json URL
return absoluteUrl.replace(/xml/g, "json");
}

All references will need to be updated as well. Anything that tests specifically that function alone (or its behavior of rewriting URLs ending in .xml->.json can be removed); though we should continue to test that functionally all SSPs from oscal-content repository continue to display properly.

@laurelmay laurelmay added bug Something isn't working and removed chore Cleanup or repository management work labels Aug 30, 2022
@brian-comply0
Copy link
Contributor

@kylelaker Ultimately referencing a XML OSCAL file from a JSON OSCAL file's import statement is valid and should be handled by our tool.

We can't count on every OSCAL file existing in both XML and JSON. Having the tool simply change the ".xml" to ".json" in the file is a fair work-around specifically for the NIST issue.

Ultimately the tool should recognize the XML extension and convert the XML file to JSON, then process it. I suppose as a shortcut, it could look for a JSON version as described in the workaround above before converting; and only convert if no JSON file was found.

In any case, I believe this issue, as written, has been addressed and should be closed. A new issue should cover the more complete functionality.

@brian-comply0
Copy link
Contributor

Closing as discussed with @kylelaker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants