Skip to content

Commit

Permalink
Merge pull request #161 from edgi-govdata-archiving/158-handle-the-re…
Browse files Browse the repository at this point in the history
…ally-real-actual-diff-data

Handle new and old diff formats
  • Loading branch information
lightandluck authored Dec 20, 2017
2 parents da2a8bd + 783b510 commit f09c579
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 61 deletions.
4 changes: 2 additions & 2 deletions src/components/changes-only-diff.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const maxContextLineLength = 300;
* @class ChangesOnlyDiff
* @extends {React.Component}
* @param {Object} props
* @param {ChangeDiff} props.diff
* @param {DiffData} props.diffData
* @param {string} props.className
*/
export default class ChangesOnlyDiff extends React.Component {
Expand All @@ -22,7 +22,7 @@ export default class ChangesOnlyDiff extends React.Component {
return null;
}

const changesOnly = this.props.diff.content.diff.reduce(
const changesOnly = this.props.diffData.diff.reduce(
getContextualDiff, []);

return (
Expand Down
47 changes: 18 additions & 29 deletions src/components/diff-view.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ import ChangesOnlyDiff from './changes-only-diff';
export default class DiffView extends React.Component {
constructor (props) {
super(props);
this.state = {diff: null};
this.state = {diffData: null};
}

componentWillMount () {
const {props} = this;
if (this._canFetch(props)) {
this._loadDiff(props.page.uuid, props.a.uuid, props.b.uuid, props.diffType);
this._loadDiffData(props.page.uuid, props.a.uuid, props.b.uuid, props.diffType);
}
}

Expand All @@ -42,64 +42,53 @@ export default class DiffView extends React.Component {
*/
componentWillReceiveProps (nextProps) {
if (this._canFetch(nextProps) && !this._propsSpecifySameDiff(nextProps)) {
this._loadDiff(nextProps.page.uuid, nextProps.a.uuid, nextProps.b.uuid, nextProps.diffType);
this._loadDiffData(nextProps.page.uuid, nextProps.a.uuid, nextProps.b.uuid, nextProps.diffType);
}
}

render () {
const {diffType} = this.props;
const {diff} = this.state;

if (diffType && diffTypes[diffType].diffService === 'TODO') {
return (
<p className="alert alert-warning" role="alert">
No diff for '{diffTypes[diffType].description}' yet
</p>
);
}

if (!diff) {
if (!this.state.diffData) {
return <Loading />;
}
else if (diff instanceof Error) {
else if (this.state.diffData instanceof Error) {
return (
<p className="alert alert-danger" role="alert">
Error: {diff.message}
Error: {this.state.diffData.message}
</p>
);
}

// TODO: if we have multiple ways to render content from a single service
// in the future (e.g. inline vs. side-by-side text), we need a better
// way to ensure we use the correct rendering and avoid race conditions
switch (diffType) {
switch (this.props.diffType) {
case diffTypes.HIGHLIGHTED_RENDERED.value:
return (
<InlineRenderedDiff diff={diff} page={this.props.page} />
<InlineRenderedDiff diffData={this.state.diffData} page={this.props.page} />
);
case diffTypes.SIDE_BY_SIDE_RENDERED.value:
return (
<SideBySideRenderedDiff diff={diff} page={this.props.page} />
<SideBySideRenderedDiff diffData={this.state.diffData} page={this.props.page} />
);
case diffTypes.OUTGOING_LINKS.value:
return (
<InlineRenderedDiff diff={diff} page={this.props.page} />
<InlineRenderedDiff diffData={this.state.diffData} page={this.props.page} />
);
case diffTypes.HIGHLIGHTED_TEXT.value:
return (
<HighlightedTextDiff diff={diff} className='diff-text-inline' />
<HighlightedTextDiff diffData={this.state.diffData} className='diff-text-inline' />
);
case diffTypes.HIGHLIGHTED_SOURCE.value:
return (
<HighlightedTextDiff diff={diff} className='diff-source-inline' />
<HighlightedTextDiff diffData={this.state.diffData} className='diff-source-inline' />
);
case diffTypes.CHANGES_ONLY_TEXT.value:
return (
<ChangesOnlyDiff diff={diff} className='diff-text-inline' />
<ChangesOnlyDiff diffData={this.state.diffData} className='diff-text-inline' />
);
case diffTypes.CHANGES_ONLY_SOURCE.value:
return (
<ChangesOnlyDiff diff={diff} className='diff-source-inline' />
<ChangesOnlyDiff diffData={this.state.diffData} className='diff-source-inline' />
);
default:
return null;
Expand Down Expand Up @@ -132,19 +121,19 @@ export default class DiffView extends React.Component {
return (props.page.uuid && props.diffType && props.a && props.b && props.a.uuid && props.b.uuid);
}

_loadDiff (pageId, aId, bId, diffType) {
_loadDiffData (pageId, aId, bId, diffType) {
// TODO - this seems to be some sort of caching mechanism, would be smart to have this for diffs
// const fromList = this.props.pages && this.props.pages.find(
// (page: Page) => page.uuid === pageId);
// Promise.resolve(fromList || this.context.api.getDiff(pageId, aId, bId, changeDiffTypes[diffType]))
this.setState({diff: null});
this.setState({diffData: null});
this.context.api.getDiff(pageId, aId, bId, diffTypes[diffType].diffService)
.catch(error => {
return error;
})
.then((diff) => {
.then((data) => {
this.setState({
diff: diff
diffData: data
});
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/highlighted-text-diff.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import List from './list';
* @class HighlightedTextDiff
* @extends {React.Component}
* @param {Object} props
* @param {ChangeDiff} props.diff
* @param {DiffData} props.diffData
* @param {string} props.className
*/
export default class HighlightedTextDiff extends React.Component {
Expand All @@ -19,7 +19,7 @@ export default class HighlightedTextDiff extends React.Component {

return (
<List
data={this.props.diff.content.diff}
data={this.props.diffData.diff}
component={DiffItem}
className={this.props.className}
/>
Expand Down
6 changes: 3 additions & 3 deletions src/components/inline-rendered-diff.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import SandboxedHtml from './sandboxed-html';

/**
* @typedef {Object} InlineRenderedDiffProps
* @property {Diff} diff The diff to render
* @property {DiffData} diffData Object containing diff to render and its metadata
* @property {Page} page The page this diff pertains to
*/

Expand All @@ -12,13 +12,13 @@ import SandboxedHtml from './sandboxed-html';
*
* @class InlineRenderedDiff
* @extends {React.Component}
* @params {InlineRenderedDiffProps} props
* @param {InlineRenderedDiffProps} props
*/
export default class InlineRenderedDiff extends React.Component {
render () {
return (
<div className="inline-render">
<SandboxedHtml html={this.props.diff.content.diff} baseUrl={this.props.page.url} />
<SandboxedHtml html={this.props.diffData.diff} baseUrl={this.props.page.url} />
</div>
);
}
Expand Down
1 change: 0 additions & 1 deletion src/components/select-diff-type.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {diffTypes} from '../constants/diff-types';
*/
export default class SelectDiffType extends React.Component {
render () {
// const diffTypes = this.props.diffTypes;
const handleChange = (event) => {
this.props.onChange(event.target.value);
};
Expand Down
8 changes: 4 additions & 4 deletions src/components/side-by-side-rendered-diff.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const showAdditions = showType.bind(null, 'additions');

/**
* @typedef {Object} SideBySideRenderedDiffProps
* @property {Diff} diff The diff to render
* @property {DiffData} diffData Object containing diff to render and its metadata
* @property {Page} page The page this diff pertains to
*/

Expand All @@ -15,19 +15,19 @@ const showAdditions = showType.bind(null, 'additions');
*
* @class SideBySideRenderedDiff
* @extends {React.Component}
* @params {SideBySideRenderedDiffProps} props
* @param {SideBySideRenderedDiffProps} props
*/
export default class SideBySideRenderedDiff extends React.Component {
render () {
return (
<div className="side-by-side-render">
<SandboxedHtml
html={this.props.diff.content.diff}
html={this.props.diffData.diff}
baseUrl={this.props.page.url}
transform={showRemovals}
/>
<SandboxedHtml
html={this.props.diff.content.diff}
html={this.props.diffData.diff}
baseUrl={this.props.page.url}
transform={showAdditions}
/>
Expand Down
12 changes: 6 additions & 6 deletions src/constants/diff-types.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
export const diffTypes = {
HIGHLIGHTED_TEXT: {
description: 'Highlighted Text',
diffService: 'html_text',
diffService: 'html_text_dmp',
},
HIGHLIGHTED_SOURCE: {
description: 'Highlighted Source',
diffService: 'html_source',
diffService: 'html_source_dmp',
},
HIGHLIGHTED_RENDERED: {
description: 'Highlighted Rendered',
diffService: 'html_visual',
diffService: 'html_token',
},
SIDE_BY_SIDE_RENDERED: {
description: 'Side-by-Side Rendered',
diffService: 'html_visual'
diffService: 'html_token'
},
OUTGOING_LINKS: {
description: 'Outgoing Links',
diffService: 'links'
},
CHANGES_ONLY_TEXT: {
description: 'Changes Only Text',
diffService: 'html_text',
diffService: 'html_text_dmp',
},
CHANGES_ONLY_SOURCE: {
description: 'Changes Only Source',
diffService: 'html_source',
diffService: 'html_source_dmp',
}
};

Expand Down
25 changes: 11 additions & 14 deletions src/services/web-monitoring-db.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,11 @@ const storageLocation = 'WebMonitoringDb.token';
*/

/**
* @typedef {Object} ChangeDiff
* @property {string} page_id
* @property {string} from_version_id
* @property {string} to_version_id
* @property {string} diff_service
* @property {string} diff_service_version
* @property {*} content
* @typedef {Object} DiffData
* @property {number} change_count
* @property {string} diff
* @property {string} version
* @property {string} type
*/

/**
Expand Down Expand Up @@ -226,7 +224,7 @@ export default class WebMonitoringDb {
* @param {string} aId
* @param {string} bId
* @param {string} diffType
* @returns {Promise<ChangeDiff>}
* @returns {Promise<DiffData>}
*/
getDiff (pageId, aId, bId, diffType) {
return this._request(this._createUrl(`pages/${pageId}/changes/${aId}..${bId}/diff/${diffType}`, {format: 'json'}))
Expand Down Expand Up @@ -404,12 +402,11 @@ function parseChange (data) {

function parseDiff (data) {
// temporarily massage old diff format into new diff format
if (data.content && data.content.data) {
let objectFormat = data.content.data;
let arrayFormat = objectFormat.map(value => {
return [value.Type, value.Text];
});
data.content = {diff: arrayFormat};
if (data.content && data.content.diff) {
return {
'diff': data.content.diff,
'change_count': null
};
}
return data;
}
Expand Down

0 comments on commit f09c579

Please sign in to comment.