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

Refactor to use API for data access #755

Closed
sebgie opened this issue Sep 15, 2013 · 7 comments · Fixed by #767
Closed

Refactor to use API for data access #755

sebgie opened this issue Sep 15, 2013 · 7 comments · Fixed by #767
Assignees
@sebgie
Milestone
0.4

Comments

@sebgie
Copy link
Contributor

sebgie commented Sep 15, 2013

One step towards #360 (Big fat refactor). Refactor ghost.js so that the following data is available with functions in api.js only:

  • config
  • settings
  • dataprovider

SettingsCache, ghost.config() and ghost.dataProvider should be moved to api while keeping initialization and migrations working.
@ghost ghost assigned sebgie Sep 15, 2013
sebgie added a commit to sebgie/Ghost that referenced this issue Sep 16, 2013
@sebgie
Escape RSS post title
141361c 
closes TryGhost#755
- added html escape for post title
- changed author for rss feed to current user
- added simple test to check rss is working
@sebgie sebgie mentioned this issue Sep 16, 2013
Merged
@ErisDS ErisDS reopened this Sep 16, 2013
@ErisDS
Copy link
Member

ErisDS commented Nov 4, 2013

Thinking about this - should the config really be behind the api? It's read only and environment specific.

@ErisDS ErisDS mentioned this issue Nov 4, 2013
Closed
@sebgie sebgie mentioned this issue Nov 10, 2013
Closed
8 tasks
@hswolff hswolff mentioned this issue Nov 28, 2013
Merged
@sebgie
Copy link
Contributor Author

sebgie commented Nov 29, 2013

I have uploaded a WIP branch to https://github.com/sebgie/Ghost/tree/issue%23755 to keep everyone updated on what is going on. It is not finished and I'm not totally happy with it yet. Any feedback/thoughts welcome!

cc @hswolff

@ErisDS
Copy link
Member

ErisDS commented Nov 30, 2013

Looks like a bloody good start to me. If it works and tests are passing, I would PR it now and improve later?

Note: Cos lots of people are working on refactors, smaller more frequent PRs are useful to help keep everyone up to date with each others' work.

@hswolff
Copy link
Contributor

hswolff commented Dec 2, 2013

Added a couple of comments on that commit @sebgie. It looks really really really f'n awesome. I'm super jazzed about it, and can't wait to have it finished and merged upstream. I already have 3 things in mind that I want to do once that's merged in:

F'n rock star. 🤘

sebgie added a commit to sebgie/Ghost that referenced this issue Dec 6, 2013
@sebgie
remove ghost.settings and ghost.notifications
078f464 
covers 90% of TryGhost#755
- moved ghost.settings to api.settings
- moved ghost.notifications to api.notifications
- split up api/index.js to notifications.js, posts.js, settings.js,
tags.js and users.js
- added instance.globals as temp workaround for blogglobals (Known
issue: blog title and blog description are updated after restart only)
- added webroot to config() to remove `var root = ...`
- changed `e` and `url` helper to async
- updated tests
@sebgie sebgie mentioned this issue Dec 6, 2013
Merged
@ErisDS
Copy link
Member

ErisDS commented Dec 7, 2013

Now that #1627 is merged, is there anything left to do here?

@sebgie
Copy link
Contributor Author

sebgie commented Dec 9, 2013

You can close the issue and I'm going to open a new issue for the remaining 10%. In fact, I would like to go though the code and remove direct access to the models everywhere possible and use data API instead.

@ErisDS
Copy link
Member

ErisDS commented Dec 9, 2013

OK cool 👍

@ErisDS ErisDS closed this as completed Dec 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sebgie sebgie

Labels
None yet
Projects
None yet
Milestone
0.4
Development

Successfully merging a pull request may close this issue.

Escape RSS post title sebgie/Ghost
3 participants
@ErisDS @hswolff @sebgie