Merge pull request #984 from zalando/feature/gh-593-permalinks

zeitlinger · web-flow · commit 7f872fea6ed7 · 2019-06-03T13:30:07.000+02:00
Permalinks UI
diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt b/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt
@@ -25,7 +25,7 @@ import javax.persistence.OneToMany
 class ApiReview(
     request: ApiDefinitionRequest,
     val userAgent: String = "",
-    @Suppress("CanBeParameter") private val apiDefinition: String,
+    @Suppress("CanBeParameter") val apiDefinition: String,
     violations: List<Result> = emptyList(),
     val name: String? = OpenApiHelper.extractApiName(apiDefinition),
     val apiId: String? = OpenApiHelper.extractApiId(apiDefinition),
diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt
@@ -101,6 +101,7 @@ class ApiViolationsController(
             Severity.SHOULD to review.shouldViolations,
             Severity.MAY to review.mayViolations,
             Severity.HINT to review.hintViolations
-        ).map { it.first.name.toLowerCase() to it.second }.toMap()
+        ).map { it.first.name.toLowerCase() to it.second }.toMap(),
+        apiDefinition = review.apiDefinition
     )
 }
diff --git a/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt b/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt
@@ -6,5 +6,6 @@ data class ApiDefinitionResponse(
     val externalId: UUID? = null,
     val message: String? = null,
     val violations: List<ViolationDTO> = emptyList(),
-    val violationsCount: Map<String, Int> = emptyMap()
+    val violationsCount: Map<String, Int> = emptyMap(),
+    val apiDefinition: String?
 )
diff --git a/server/src/main/resources/api/zally-api.yaml b/server/src/main/resources/api/zally-api.yaml
@@ -288,6 +288,9 @@ components:
           description: Custom server message
         violations_count:
           $ref: '#/components/schemas/ViolationsCount'
+        api_definition:
+          type: string
+          description: Specification object in OpenAPI format
 
     SupportedRulesResponse:
       type: object
diff --git a/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt b/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt
@@ -2,19 +2,24 @@ package de.zalando.zally.apireview
 
 import de.zalando.zally.configuration.JacksonObjectMapperConfiguration
 import de.zalando.zally.dto.ApiDefinitionRequest
+import org.hamcrest.CoreMatchers.hasItem
 import org.hamcrest.Matchers.containsString
+import org.hamcrest.Matchers.notNullValue
 import org.intellij.lang.annotations.Language
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
 import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.http.MediaType.APPLICATION_JSON_UTF8
 import org.springframework.test.context.ActiveProfiles
 import org.springframework.test.context.junit4.SpringRunner
 import org.springframework.test.web.servlet.MockMvc
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 
 @RunWith(SpringRunner::class)
@@ -36,7 +41,13 @@ class ApiViolationsControllerTest {
                 .content("{\"api_definition_string\":\"\"}")
         )
             .andExpect(status().isOk)
+            .andExpect(header().exists("Location"))
+            .andExpect(content().contentType(APPLICATION_JSON_UTF8))
             .andExpect(content().string(containsString("https://zalando.github.io/restful-api-guidelines")))
+            .andExpect(jsonPath("$.violations[*].rule_link", hasItem("https://zalando.github.io/restful-api-guidelines/#101")))
+            .andExpect(jsonPath("$.external_id", notNullValue()))
+            .andExpect(jsonPath("$.violations", notNullValue()))
+            .andExpect(jsonPath("$.api_definition", notNullValue()))
     }
 
     @Test
@@ -49,7 +60,7 @@ class ApiViolationsControllerTest {
     }
 
     @Test
-    fun `getExistingViolationResponse with existing responds NotFound`() {
+    fun `getExistingViolationResponse with existing responds Ok`() {
 
         val location = mvc.perform(
             post("/api-violations")
@@ -64,7 +75,10 @@ class ApiViolationsControllerTest {
                 .accept("application/json")
         )
             .andExpect(status().isOk)
-            .andExpect(content().string(containsString("https://zalando.github.io/restful-api-guidelines")))
+            .andExpect(content().contentType(APPLICATION_JSON_UTF8))
+            .andExpect(jsonPath("$.external_id", notNullValue()))
+            .andExpect(jsonPath("$.violations", notNullValue()))
+            .andExpect(jsonPath("$.api_definition", notNullValue()))
     }
 
     /**
diff --git a/web-ui/src/client/app/components/__tests__/violations.test.jsx b/web-ui/src/client/app/components/__tests__/violations.test.jsx
@@ -52,7 +52,6 @@ describe('Violations component', () => {
   test('should render empty list of violation', () => {
     const component = shallow(<Violations violations={[]} />);
     expect(component.find('Violation')).toHaveLength(0);
-    expect(component.find('h3')).toHaveLength(0);
   });
 });
 
diff --git a/web-ui/src/client/app/components/violations.jsx b/web-ui/src/client/app/components/violations.jsx
@@ -3,13 +3,21 @@ import { If } from './util.jsx';
 import { Msg } from './msg.jsx';
 import { RuleType } from './rules.jsx';
 import FluidContainer from './fluid-container.jsx';
+import { Link } from 'react-router-dom';
 
 export function Violations(props) {
   return (
     <div>
       <div className="dc-row">
         <div className="dc-column">
-          {props.violations.length ? <h3>VIOLATIONS</h3> : ''}
+          <h3>
+            VIOLATIONS
+            <span style={{ float: 'right' }}>
+              <Link to={'/editor/' + props.externalId} className="dc-link">
+                <i className="dc-icon dc-icon--interactive dc-icon--link" />
+              </Link>
+            </span>
+          </h3>
         </div>
       </div>
       <div className="violations-container">
@@ -136,6 +144,7 @@ export function ViolationsResult(props) {
         dataTestId="if-violations"
       >
         <Violations
+          externalId={props.externalId}
           violations={props.violations}
           violationsCount={props.violationsCount}
         />
diff --git a/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js b/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js
@@ -43,6 +43,16 @@ describe('ViolationsTab container component', () => {
     expect(component.find('Editor')).toHaveLength(1);
     expect(component.find('Rules')).toHaveLength(0);
   });
+  test('render the Editor tab on /editor/:externalId', () => {
+    const component = mount(
+      <StaticRouter location={{ pathname: '/editor/:externalId' }}>
+        <ViolationsTab authenticated />
+      </StaticRouter>
+    );
+    expect(component.find('URL')).toHaveLength(0);
+    expect(component.find('Editor')).toHaveLength(1);
+    expect(component.find('Rules')).toHaveLength(0);
+  });
   test('render the Rules tab on /rules', () => {
     const component = mount(
       <StaticRouter location={{ pathname: '/rules' }}>
diff --git a/web-ui/src/client/app/containers/app.jsx b/web-ui/src/client/app/containers/app.jsx
@@ -14,6 +14,7 @@ export function App(props) {
     Storage,
     getApiViolationsByURL,
     getApiViolationsBySchema,
+    getApiViolationsByExternalId,
     getSupportedRules,
     getFile,
   } = props;
@@ -58,6 +59,7 @@ export function App(props) {
                 getSupportedRules={getSupportedRules}
                 getApiViolationsByURL={getApiViolationsByURL}
                 getApiViolationsBySchema={getApiViolationsBySchema}
+                getApiViolationsByExternalId={getApiViolationsByExternalId}
                 getFile={getFile}
                 Storage={Storage}
                 {...props}
diff --git a/web-ui/src/client/app/containers/editor.jsx b/web-ui/src/client/app/containers/editor.jsx
@@ -4,6 +4,7 @@ import { Violations } from './violations.jsx';
 import { ViolationsResult } from '../components/violations.jsx';
 import { EditorInputForm } from '../components/editor.jsx';
 import { Dialog } from '../components/dialog.jsx';
+import { id } from 'brace/worker/json';
 
 export const editorErrorToAnnotations = error => {
   if (!error || !error.mark) {
@@ -22,15 +23,58 @@ export const editorErrorToAnnotations = error => {
 export class Editor extends Violations {
   constructor(props) {
     super(props);
+    if (
+      props &&
+      props.match &&
+      props.match.params &&
+      props.match.params.externalId
+    ) {
+      this.state.externalId = props.match.params.externalId;
+    } else {
+      this.state.externalId = null;
+    }
     this.state.editorDirty = true;
-    this.state.editorValue = this.Storage.getItem('editor-value') || '';
+    this.state.editorValue =
+      (!this.state.externalId && this.Storage.getItem('editor-value')) || '';
     this.handleOnInputValueChange = this.handleOnInputValueChange.bind(this);
     this.handleFormSubmit = this.handleFormSubmit.bind(this);
     this.handleHideOverlay = this.handleHideOverlay.bind(this);
   }
 
   componentDidMount() {
-    this.updateInputValue(this.state.editorValue);
+    if (this.state.externalId) {
+      this.getApiViolationsByExternalId(this.state.externalId)
+        .then(response => {
+          this.setState({
+            pending: false,
+            ajaxComplete: true,
+            externalId: response.external_id,
+            violations: response.violations,
+            violationsCount: response.violations_count,
+            editorValue: response.api_definition,
+          });
+          this.Storage.setItem('editor-value', this.state.editorValue);
+          this.updateInputValue(this.state.editorValue);
+        })
+        .catch(error => {
+          console.error(error); // eslint-disable-line no-console
+          this.setState({
+            pending: false,
+            ajaxComplete: true,
+            error: error.detail || Violations.DEFAULT_ERROR_MESSAGE,
+            violations: [],
+            violationsCount: {
+              could: 0,
+              hint: 0,
+              must: 0,
+              should: 0,
+            },
+          });
+          return Promise.reject(error);
+        });
+    } else {
+      this.updateInputValue(this.state.editorValue);
+    }
   }
 
   handleFormSubmit(event) {
@@ -95,6 +139,7 @@ export class Editor extends Violations {
                 pending={this.state.pending}
                 complete={this.state.ajaxComplete}
                 errorMsgText={this.state.error}
+                externalId={this.state.externalId}
                 violations={this.state.violations}
                 successMsgTitle={this.state.successMsgTitle}
                 successMsgText={this.state.successMsgText}
@@ -107,6 +152,7 @@ export class Editor extends Violations {
             pending={this.state.pending}
             complete={this.state.ajaxComplete}
             errorMsgText={this.state.error}
+            externalId={this.state.externalId}
             violations={this.state.violations}
             successMsgTitle={this.state.successMsgTitle}
             successMsgText={this.state.successMsgText}
diff --git a/web-ui/src/client/app/containers/root.jsx b/web-ui/src/client/app/containers/root.jsx
@@ -18,6 +18,9 @@ export function Root(props) {
         getApiViolationsBySchema={props.RestService.getApiViolationsBySchema.bind(
           props.RestService
         )}
+        getApiViolationsByExternalId={props.RestService.getApiViolationsByExternalId.bind(
+          props.RestService
+        )}
         getFile={props.RestService.getFile.bind(props.RestService)}
         Storage={props.Storage}
         user={props.user}
diff --git a/web-ui/src/client/app/containers/url.jsx b/web-ui/src/client/app/containers/url.jsx
@@ -60,6 +60,7 @@ export class URL extends Violations {
           pending={this.state.pending}
           complete={this.state.ajaxComplete}
           errorMsgText={this.state.error}
+          externalId={this.state.externalId}
           violations={this.state.violations}
           successMsgTitle={this.state.successMsgTitle}
           successMsgText={this.state.successMsgText}
diff --git a/web-ui/src/client/app/containers/violations-tab.jsx b/web-ui/src/client/app/containers/violations-tab.jsx
@@ -10,6 +10,7 @@ export function ViolationsTab({
   authenticated,
   getApiViolationsByURL,
   getApiViolationsBySchema,
+  getApiViolationsByExternalId,
   getSupportedRules,
   getFile,
   Storage,
@@ -82,6 +83,18 @@ export function ViolationsTab({
               render={props => (
                 <Editor
                   getApiViolations={getApiViolationsBySchema}
+                  getApiViolationsByExternalId={getApiViolationsByExternalId}
+                  Storage={Storage}
+                  {...props}
+                />
+              )}
+            />
+            <Route
+              path="/editor/:externalId"
+              render={props => (
+                <Editor
+                  getApiViolations={getApiViolationsBySchema}
+                  getApiViolationsByExternalId={getApiViolationsByExternalId}
                   Storage={Storage}
                   {...props}
                 />
diff --git a/web-ui/src/client/app/containers/violations.jsx b/web-ui/src/client/app/containers/violations.jsx
@@ -6,12 +6,14 @@ export class Violations extends Component {
 
     this.Storage = this.props.Storage;
     this.getApiViolations = this.props.getApiViolations;
+    this.getApiViolationsByExternalId = this.props.getApiViolationsByExternalId;
 
     this.state = {
       error: null,
       pending: false,
       ajaxComplete: false,
       inputValue: '',
+      externalId: null,
       violations: [],
       violationsCount: {
         could: 0,
@@ -34,6 +36,7 @@ export class Violations extends Component {
         this.setState({
           pending: false,
           ajaxComplete: true,
+          externalId: response.external_id,
           violations: response.violations,
           violationsCount: response.violations_count,
         });
diff --git a/web-ui/src/client/app/services/rest.js b/web-ui/src/client/app/services/rest.js
@@ -34,6 +34,25 @@ export const RestService = {
     });
   },
 
+  getApiViolationsByExternalId(externalId) {
+    const options = {
+      method: 'GET',
+      headers: {
+        Accept: 'application/json',
+        'Content-Type': 'application/json',
+      },
+    };
+    return client
+      .fetch(
+        `${
+          window.env.ZALLY_API_URL
+        }/api-violations/${externalId}`,
+        options
+      )
+      .then(response => response.json())
+      .catch(response => response.json().then(body => Promise.reject(body)));
+  },
+
   getSupportedRules() {
     const options = {
       method: 'GET',

Original file line number	Diff line number	Diff line change
`@@ -101,6 +101,7 @@ class ApiViolationsController(`
`101`	`101`	`Severity.SHOULD to review.shouldViolations,`
`102`	`102`	`Severity.MAY to review.mayViolations,`
`103`	`103`	`Severity.HINT to review.hintViolations`
`104`		`- ).map { it.first.name.toLowerCase() to it.second }.toMap()`
	`104`	`+ ).map { it.first.name.toLowerCase() to it.second }.toMap(),`
	`105`	`+ apiDefinition = review.apiDefinition`
`105`	`106`	`)`
`106`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,5 +6,6 @@ data class ApiDefinitionResponse(`
`6`	`6`	`val externalId: UUID? = null,`
`7`	`7`	`val message: String? = null,`
`8`	`8`	`val violations: List<ViolationDTO> = emptyList(),`
`9`		`- val violationsCount: Map<String, Int> = emptyMap()`
	`9`	`+ val violationsCount: Map<String, Int> = emptyMap(),`
	`10`	`+ val apiDefinition: String?`
`10`	`11`	`)`