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.

Sign up for GitHub

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

Jump to bottom

Add Create API #31

Merged
merged 12 commits into from
Jun 19, 2020
Merged

Add Create API #31

merged 12 commits into from
Jun 19, 2020

Conversation

christiemolloy
Copy link
Contributor

@christiemolloy christiemolloy commented Apr 23, 2020 •
edited
Loading

Adds the Create API Page

  • Form component
  • Validation for form components
    • Check if the name field is empty (Right now I copied what was in Apicurio-studio for validation, looks like thats the only thing being validated ? I might also add validation for certain characters? )
  • Uses the onCreateApi method + importApi methods to send the post request to the server.

Things to check:

  • Form validation works and doesn't submit the form if not validated.
  • Validation/error message works as expected
  • Check that when you create an API it works and when you re-render the new API displays

Screen Shot 2020-06-03 at 4 32 43 PM

dlabaj
dlabaj previously approved these changes Apr 28, 2020
View reviewed changes
@christiemolloy christiemolloy marked this pull request as ready for review June 3, 2020 20:33
jenny-s51
jenny-s51 reviewed Jun 8, 2020
View reviewed changes
Copy link
Contributor

@jenny-s51 jenny-s51 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks like the form validation is working! What is the expected behavior when clicking the "Create API" button? At the moment I get [CreateApiPageComponent] Creating a new (blank) API: {"type":"OpenAPI30","name":"API 1","description":"description of API 1"} printed to the console. Is there anything that should change on the UI, or is that something that will be implemented in the future?

dlabaj
dlabaj reviewed Jun 18, 2020
View reviewed changes
Copy link
Collaborator

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

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

a few comments.

package.json Outdated Show resolved Hide resolved
packages/models/src/create-api-form-data.model.ts Show resolved Hide resolved
packages/services/src/api-services/api-services.ts Show resolved Hide resolved
dlabaj
dlabaj previously approved these changes Jun 19, 2020
View reviewed changes
EricWittmann
EricWittmann reviewed Jun 19, 2020
View reviewed changes
packages/models/src/create-api-form-data.model.ts
@@ -0,0 +1,32 @@
/**
* @license
* Copyright 2017 JBoss Inc
Copy link
Member

Choose a reason for hiding this comment

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

We should probably do a find/replace from 2017 to 2020 on all these files. Or else remove the license entirely if we don't need it. I'm never sure. Either way, not something to change for this PR...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! i created this issue to track it: #37

EricWittmann
EricWittmann reviewed Jun 19, 2020
View reviewed changes
packages/services/src/api-services/hub.ts Outdated
return axios.request(config)
.then(response => {
let data = response;
console.log('what is the response' + data)
Copy link
Member

Choose a reason for hiding this comment

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

Remove this before checkin/merge?

EricWittmann
EricWittmann reviewed Jun 19, 2020
View reviewed changes
packages/services/src/api-services/hub.ts Outdated
data: stringify
}, ...options}

console.log('did it make it to the post');
Copy link
Member

Choose a reason for hiding this comment

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

Debug statement still needed?

EricWittmann
EricWittmann previously approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@EricWittmann EricWittmann left a comment

Choose a reason for hiding this comment

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

lgtm - I'm assuming the template stuff will happen in another pass.

@christiemolloy christiemolloy dismissed stale reviews from EricWittmann and dlabaj via c3959bf June 19, 2020 18:02
@christiemolloy
Copy link
Contributor Author

thanks for the review, updated @EricWittmann

@jenny-s51 jenny-s51 merged commit 5d14c71 into Apicurio:master Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenny-s51 jenny-s51 jenny-s51 approved these changes

@dlabaj dlabaj dlabaj left review comments

@EricWittmann EricWittmann EricWittmann left review comments

@mfrances17 mfrances17 Awaiting requested review from mfrances17

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@christiemolloy @EricWittmann @dlabaj @jenny-s51