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

Feature/api #132

Merged
merged 36 commits into from Jul 21, 2016
Merged

Feature/api #132

merged 36 commits into from Jul 21, 2016

Conversation

Snugug
Copy link
Member

@Snugug Snugug commented Jul 20, 2016

REST API Endpoints.

Code review can take place as I'm finishing up tests


Resolves #114

DCO 1.1 Signed-off-by: Sam Richard <sam@snug.ug>

Sam Richard and others added 28 commits July 19, 2016 11:25
This is where live content will be transfered to for APIs
* Fixes spacing for scenarios
* Moves feature for checkboxes out of the label as that gets hard to read
Formatting of content, sorting, and pagination
Cleans up work and makes it unit testable
Hopefully will stop Travis lockup
@Snugug
Copy link
Member Author

Snugug commented Jul 21, 2016

This is OK to review. The tests seem to be a little fragile because I'm randomly generating stuff. Thoughts on that?

const organize = utils.organize(query);

return database.select('*').from('live').orderBy(organize.sort.by, organize.sort.dir).offset(organize.page.offset).limit(organize.page.limit).then(rows => {
return database('live').count('id').then(total => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Case in point

@scottnath
Copy link
Contributor

OK. Review complete. Some fixes required.

Non-PR-fix action items:

  • docs: API query examples in README
  • development: jsdoc linting working?
  • story-to-write: error returns
  • cleanup: lib/init/authenticated.js: authenticated clean up caveats
    • don't worry about this - when I do workflows' auth requirements I'll prol be in this file.

@Snugug
Copy link
Member Author

Snugug commented Jul 21, 2016

@scottnath

  • None of Punchcard's functionality is documented in the README. I'm not sure if adding API documentation there makes sense, but we do eventually need to document Punchcard's functionality
  • Updated, take a look

@scottnath
Copy link
Contributor

@Snugug failed test:

apis › APIs: Types
            operator: ===
            expected: [object Object],[object Object],[object Object],[object Object],[object Object]
            actual: [object Object],[object Object],[object Object],[object Object],[object Object]

@Snugug
Copy link
Member Author

Snugug commented Jul 21, 2016

This is the test that sometimes and sometimes does not fail. I may need to re-write this test

Sam Richard added 3 commits July 21, 2016 15:07
Damnit, it worked with just logging stuff out. If I don't, will it?
Trying this again. I'm going to be _so_ mad if this works
@Snugug
Copy link
Member Author

Snugug commented Jul 21, 2016

@scottnath I frickin give up. logging out the things that need to be identical makes the tests pass, but not having them logged out makes it fail. So ya know what, I'm keeping the logging in

@scottnath scottnath merged commit a109530 into punchcard-cms:master Jul 21, 2016
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.

None yet

3 participants