From df9fc91123ca5aa5fb78187cb6864678072b5362 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 15 Jan 2018 17:25:31 +0530 Subject: [PATCH] routing for desk, error handling, test for router --- backends/rest_client.js | 6 +-- client/desk/index.js | 65 +++++++++++++++++++++--- client/desk/router.js | 82 ------------------------------ client/index.js | 6 ++- client/view/form.js | 8 ++- client/view/list.js | 6 +-- client/view/page.js | 35 ++++++++----- common/errors.js | 21 ++++++++ common/index.js | 2 + common/router.js | 99 +++++++++++++++++++++++++++++++++++++ docs/client/desk.md | 27 ++++++++++ docs/errors.md | 16 ++++++ index.js | 46 ++++++++++++++--- model/document.js | 7 ++- model/meta.js | 2 +- models/doctype/todo/todo.js | 5 +- server/index.js | 4 -- tests/test_document.js | 2 +- tests/test_rest_api.js | 4 +- tests/test_router.js | 28 +++++++++++ utils/index.js | 5 +- 21 files changed, 339 insertions(+), 137 deletions(-) delete mode 100644 client/desk/router.js create mode 100644 common/errors.js create mode 100644 common/router.js create mode 100644 docs/client/desk.md create mode 100644 docs/errors.md create mode 100644 tests/test_router.js diff --git a/backends/rest_client.js b/backends/rest_client.js index 0b623d68..801c6a6e 100644 --- a/backends/rest_client.js +++ b/backends/rest_client.js @@ -1,7 +1,7 @@ const frappe = require('frappe-core'); const path = require('path'); -class RESTClient { +module.exports = class RESTClient { constructor({server, protocol='http'}) { this.server = server; this.protocol = protocol; @@ -123,8 +123,4 @@ class RESTClient { } -} - -module.exports = { - Database: RESTClient } \ No newline at end of file diff --git a/client/desk/index.js b/client/desk/index.js index 67bb464c..cd31c91a 100644 --- a/client/desk/index.js +++ b/client/desk/index.js @@ -1,10 +1,14 @@ const frappe = require('frappe-core'); const Search = require('./search'); -const Router = require('./router'); +const Router = require('frappe-core/common/router'); +const Page = require('frappe-core/client/view/page'); +const List = require('frappe-core/client/view/list'); +const Form = require('frappe-core/client/view/form'); module.exports = class Desk { constructor() { frappe.router = new Router(); + frappe.router.listen(); this.wrapper = document.querySelector('.desk'); @@ -15,20 +19,67 @@ module.exports = class Desk { this.main = frappe.ui.add('div', 'main', this.body); this.sidebar_items = []; - this.list_pages = {}; - this.edit_pages = {}; + this.pages = { + lists: {}, + forms: {} + }; + + this.init_routes(); // this.search = new Search(this.nav); } init_routes() { - frappe.router.on('list/:doctype', (params) => { - - }) - frappe.router.on('edit/:doctype/:name', (params) => { + frappe.router.add('list/:doctype', async (params) => { + let page = this.get_list_page(params.doctype); + await page.show(params); + }); + frappe.router.add('edit/:doctype/:name', async (params) => { + let page = this.get_form_page(params.doctype); + await page.show(params); }) + frappe.router.add('new/:doctype', async (params) => { + let doc = await frappe.get_new_doc(params.doctype); + frappe.router.set_route('edit', doc.doctype, doc.name); + }); + + } + + get_list_page(doctype) { + if (!this.pages.lists[doctype]) { + let page = new Page('List ' + doctype); + page.list = new List({ + doctype: doctype, + parent: page.body + }); + page.on('show', async () => { + await page.list.run(); + }); + this.pages.lists[doctype] = page; + } + return this.pages.lists[doctype]; + } + + get_form_page(doctype) { + if (!this.pages.forms[doctype]) { + let page = new Page('Edit ' + doctype); + page.form = new Form({ + doctype: doctype, + parent: page.body + }); + page.on('show', async (params) => { + try { + page.doc = await frappe.get_doc(params.doctype, params.name); + page.form.use(page.doc); + } catch (e) { + page.render_error(e.status_code, e.message); + } + }); + this.pages.forms[doctype] = page; + } + return this.pages.forms[doctype]; } add_sidebar_item(label, action) { diff --git a/client/desk/router.js b/client/desk/router.js deleted file mode 100644 index 2fef1a2c..00000000 --- a/client/desk/router.js +++ /dev/null @@ -1,82 +0,0 @@ -const frappe = require('frappe-core'); - -module.exports = class Router { - constructor() { - this.current_page = null; - this.static_routes = {}; - this.dynamic_routes = {}; - this.listen(); - } - - add(route, handler) { - let page = {handler: handler}; - - // '/todo/:name/:place'.match(/:([^/]+)/g); - page.param_keys = route.match(/:([^/]+)/g); - - if (page.param_keys) { - // make expression - // '/todo/:name/:place'.replace(/\/:([a-z1-9]+)/g, "\/([a-z0-9]+)"); - page.expression = route.replace(/\/:([a-z1-9]+)/g, "\/([a-z0-9]+)"); - this.dynamic_routes[route] = page; - } else { - this.static_routes[route] = page; - } - - } - - listen() { - window.onhashchange = this.changed.bind(this); - this.changed(); - } - - async changed(event) { - if (window.location.hash.length > 0) { - const page_name = window.location.hash.substr(1); - this.show(page_name); - } else if (this.static_routes['default']) { - this.show('default'); - } - } - - show(route) { - if (!route) { - route = 'default'; - } - - if (route[0]==='#') { - route = route.substr(1); - } - - let page = this.match(route); - - if (page) { - if (typeof page.handler==='function') { - page.handler(page.params); - } else { - page.handler.show(page.params); - } - } - } - - match(route) { - for(let key in this.static_routes) { - if (key === route) { - return {handler: this.static_routes[key].handler}; - } - } - - for(let key in this.dynamic_routes) { - let page = this.dynamic_routes[key]; - let matches = route.match(new RegExp(page.expression)); - - if (matches && matches.length == page.param_keys.length + 1) { - let params = {} - for (let i=0; i < page.param_keys.length; i++) { - params[page.param_keys[i].substr(1)] = matches[i + 1]; - } - return {handler:page.handler, params: params}; - } - } - } -} \ No newline at end of file diff --git a/client/index.js b/client/index.js index e6c41e94..6b988c7a 100644 --- a/client/index.js +++ b/client/index.js @@ -1,5 +1,5 @@ const common = require('frappe-core/common'); -const Database = require('frappe-core/backends/rest_client').Database; +const RESTClient = require('frappe-core/backends/rest_client'); const frappe = require('frappe-core'); frappe.ui = require('./ui'); const Desk = require('./desk'); @@ -11,7 +11,9 @@ module.exports = { common.init_libs(frappe); frappe.fetch = window.fetch.bind(); - frappe.db = await new Database({server: server}); + frappe.db = await new RESTClient({server: server}); + + frappe.flags.cache_docs = true; frappe.desk = new Desk(); await frappe.login(); diff --git a/client/view/form.js b/client/view/form.js index a6c12ae0..0eb5de9d 100644 --- a/client/view/form.js +++ b/client/view/form.js @@ -1,7 +1,7 @@ const frappe = require('frappe-core'); const controls = require('./controls'); -class Form { +module.exports = class Form { constructor({doctype, parent, submit_label='Submit'}) { this.parent = parent; this.doctype = doctype; @@ -85,7 +85,7 @@ class Form { async submit() { try { - if (this.is_new) { + if (this.is_new || this.doc.__not_inserted) { await this.doc.insert(); } else { await this.doc.update(); @@ -103,6 +103,4 @@ class Form { } } -} - -module.exports = {Form: Form}; \ No newline at end of file +} \ No newline at end of file diff --git a/client/view/list.js b/client/view/list.js index d2a3c15d..3d66085c 100644 --- a/client/view/list.js +++ b/client/view/list.js @@ -1,6 +1,6 @@ const frappe = require('frappe-core'); -class ListView { +module.exports = class List { constructor({doctype, parent, fields}) { this.doctype = doctype; this.parent = parent; @@ -117,8 +117,4 @@ class ListView { } } -}; - -module.exports = { - ListView: ListView }; \ No newline at end of file diff --git a/client/view/page.js b/client/view/page.js index e5d79f01..dbfd72e8 100644 --- a/client/view/page.js +++ b/client/view/page.js @@ -1,6 +1,6 @@ const frappe = require('frappe-core'); -class Page { +module.exports = class Page { constructor(title) { this.handlers = {}; this.title = title; @@ -8,12 +8,12 @@ class Page { } make() { - this.body = frappe.ui.add('div', 'page hide', frappe.desk.main); + this.wrapper = frappe.ui.add('div', 'page hide', frappe.desk.main); + this.body = frappe.ui.add('div', 'page-body', this.wrapper); } hide() { - frappe.ui.add_class(this.body, 'hide'); - + this.wrapper.classList.add('hide'); this.trigger('hide'); } @@ -21,25 +21,38 @@ class Page { if (frappe.router.current_page) { frappe.router.current_page.hide(); } - frappe.ui.remove_class(this.body, 'hide'); + this.wrapper.classList.remove('hide'); + this.body.classList.remove('hide'); + + if (this.page_error) { + this.page_error.classList.add('hide'); + } + frappe.router.current_page = this; document.title = this.title; this.trigger('show', params); } + render_error(status_code, message) { + if (!this.page_error) { + this.page_error = frappe.ui.add('div', 'page-error', this.wrapper); + } + this.body.classList.add('hide'); + this.page_error.classList.remove('hide'); + this.page_error.innerHTML = `

${status_code}

${message}

`; + } + on(event, fn) { - if (!this.handlers[event]) this.handlers.event = []; + if (!this.handlers[event]) this.handlers[event] = []; this.handlers[event].push(fn); } - trigger(event, params) { + async trigger(event, params) { if (this.handlers[event]) { for (let handler of this.handlers[event]) { - handler(params); + await handler(params); } } } -} - -module.exports = { Page: Page }; \ No newline at end of file +} \ No newline at end of file diff --git a/common/errors.js b/common/errors.js new file mode 100644 index 00000000..6dce6b1a --- /dev/null +++ b/common/errors.js @@ -0,0 +1,21 @@ +class BaseError extends Error { + constructor(status_code, ...params) { + super(...params); + this.status_code = status_code; + } +} + +class ValidationError extends BaseError { + constructor(...params) { super(417, ...params); } +} + +module.exports = { + ValidationError: ValidationError, + ValueError: class ValueError extends ValidationError { }, + NotFound: class NotFound extends BaseError { + constructor(...params) { super(404, ...params); } + }, + Forbidden: class Forbidden extends BaseError { + constructor(...params) { super(403, ...params); } + }, +} diff --git a/common/index.js b/common/index.js index 608f3b90..e0a49efc 100644 --- a/common/index.js +++ b/common/index.js @@ -4,6 +4,7 @@ const model = require('../model'); const _document = require('../model/document'); const meta = require('../model/meta'); const _session = require('../session'); +const errors = require('./errors'); module.exports = { @@ -14,5 +15,6 @@ module.exports = { frappe.document = _document; frappe.meta = meta; frappe._session = _session; + frappe.errors = errors; } } \ No newline at end of file diff --git a/common/router.js b/common/router.js new file mode 100644 index 00000000..47c7bf93 --- /dev/null +++ b/common/router.js @@ -0,0 +1,99 @@ +const frappe = require('frappe-core'); + +module.exports = class Router { + constructor() { + this.current_page = null; + this.static_routes = []; + this.dynamic_routes = []; + } + + add(route, handler) { + let page = {handler: handler, route: route}; + + // '/todo/:name/:place'.match(/:([^/]+)/g); + page.param_keys = route.match(/:([^/]+)/g); + + if (page.param_keys) { + // make expression + // '/todo/:name/:place'.replace(/\/:([a-z1-9]+)/g, "\/([a-z0-9]+)"); + page.expression = route.replace(/\/:([a-z1-9]+)/g, "\/([a-z0-9]+)"); + this.dynamic_routes.push(page); + this.sort_dynamic_routes(); + } else { + this.static_routes.push(page); + this.sort_static_routes(); + } + } + + sort_dynamic_routes() { + // routes with fewer parameters first + this.dynamic_routes = this.dynamic_routes.sort((a, b) => { + if (a.param_keys.length > b.param_keys.length) { + return 1; + } else if (a.param_keys.length < b.param_keys.length) { + return -1; + } else { + return a.route.length > b.route.length ? 1 : -1; + } + }) + } + + sort_static_routes() { + // longer routes on first + this.static_routes = this.static_routes.sort((a, b) => { + return a.route.length > b.route.length ? 1 : -1; + }); + } + + listen() { + window.addEventListener('hashchange', (event) => { + this.show(window.location.hash); + }); + } + + set_route(...parts) { + const route = parts.join('/'); + window.location.hash = route; + } + + async show(route) { + if (route && route[0]==='#') { + route = route.substr(1); + } + + if (!route) { + route = this.default; + } + let page = this.match(route); + + if (page) { + if (typeof page.handler==='function') { + await page.handler(page.params); + } else { + await page.handler.show(page.params); + } + } + } + + match(route) { + // match static + for(let page of this.static_routes) { + if (page.route === route) { + return {handler: page.handler}; + } + } + + // match dynamic + for(let page of this.dynamic_routes) { + let matches = route.match(new RegExp(page.expression)); + + if (matches && matches.length == page.param_keys.length + 1) { + let params = {} + for (let i=0; i < page.param_keys.length; i++) { + params[page.param_keys[i].substr(1)] = matches[i + 1]; + } + return {handler:page.handler, params: params}; + } + } + } +} \ No newline at end of file diff --git a/docs/client/desk.md b/docs/client/desk.md new file mode 100644 index 00000000..e1e4ad1d --- /dev/null +++ b/docs/client/desk.md @@ -0,0 +1,27 @@ +# Desk + +Desk includes the default routing and menu system for the single page application + +## Menus + +You can add a new menu to the desk via + +```js +frappe.desk.add_sidebar_item('New ToDo', '#new/todo'); +``` + +## Views + +Default route handling for various views + +### List Documents + +All list views are rendered at `/list/:doctype` + +### Edit Documents + +Documents can be edited via `/edit/:doctype/:name` + +### New Documents + +New Documents can be created via `/new/:doctype` \ No newline at end of file diff --git a/docs/errors.md b/docs/errors.md new file mode 100644 index 00000000..6f5ec575 --- /dev/null +++ b/docs/errors.md @@ -0,0 +1,16 @@ +# Errors + +Frappe.js comes with standard error classes that have an HTTP status code attached. + +For example you can raise a "not found" (HTTP Status Code 404) via: + +```js +throw new frappe.errors.NotFound('Document Not Found'); +``` + +### Standard Errors + +- 403: Forbidden +- 404: NotFound +- 417: ValidationError +- 417: ValueError \ No newline at end of file diff --git a/index.js b/index.js index dd1bb857..724131a4 100644 --- a/index.js +++ b/index.js @@ -2,7 +2,6 @@ module.exports = { async init() { if (this._initialized) return; this.init_config(); - this.init_errors(); this.init_globals(); this._initialized = true; }, @@ -14,12 +13,30 @@ module.exports = { }; }, - init_errors() { - this.ValueError = class extends Error { }; - }, - init_globals() { this.meta_cache = {}; + this.docs = {}; + this.flags = { + cache_docs: false + } + }, + + add_to_cache(doc) { + if (!this.flags.cache_docs) return; + + // add to `docs` cache + if (doc.doctype && doc.name) { + if (!this.docs[doc.doctype]) { + this.docs[doc.doctype] = {}; + } + this.docs[doc.doctype][doc.name] = doc; + } + }, + + get_doc_from_cache(doctype, name) { + if (this.docs[doctype] && this.docs[doctype][name]) { + return this.docs[doctype][name]; + } }, get_meta(doctype) { @@ -37,9 +54,14 @@ module.exports = { async get_doc(data, name) { if (typeof data==='string' && typeof name==='string') { - let controller_class = this.models.get_controller(data); - var doc = new controller_class({doctype:data, name: name}); - await doc.load(); + let doc = this.get_doc_from_cache(data, name); + if (!doc) { + let controller_class = this.models.get_controller(data); + doc = new controller_class({doctype:data, name: name}); + await doc.load(); + this.add_to_cache(doc); + } + return doc; } else { let controller_class = this.models.get_controller(data.doctype); var doc = new controller_class(data); @@ -47,6 +69,14 @@ module.exports = { return doc; }, + async get_new_doc(doctype) { + let doc = await frappe.get_doc({doctype: doctype}); + doc.set_name(); + doc.__not_inserted = true; + this.add_to_cache(doc); + return doc; + }, + async insert(data) { const doc = await this.get_doc(data); return await doc.insert(); diff --git a/model/document.js b/model/document.js index 1f73d43b..fa6c188c 100644 --- a/model/document.js +++ b/model/document.js @@ -98,7 +98,12 @@ class Document { } async load() { - Object.assign(this, await frappe.db.get(this.doctype, this.name)); + let data = await frappe.db.get(this.doctype, this.name); + if (data.name) { + Object.assign(this, data); + } else { + throw new frappe.errors.NotFound(`Not Found: ${this.doctype} ${this.name}`); + } } async insert() { diff --git a/model/meta.js b/model/meta.js index 900844f2..7e74836d 100644 --- a/model/meta.js +++ b/model/meta.js @@ -77,7 +77,7 @@ class Meta extends Document { options = df.options.split('\n'); } if (!options.includes(value)) { - throw new frappe.ValueError(`${value} must be one of ${options.join(", ")}`); + throw new frappe.errors.ValueError(`${value} must be one of ${options.join(", ")}`); } } diff --git a/models/doctype/todo/todo.js b/models/doctype/todo/todo.js index e4124387..734060a0 100644 --- a/models/doctype/todo/todo.js +++ b/models/doctype/todo/todo.js @@ -4,11 +4,12 @@ class todo_meta extends frappe.meta.Meta { setup_meta() { Object.assign(this, require('./todo.json')); this.name = 'ToDo'; - this.list_options.fields = ['name', 'subject', 'status', 'description']; + this.list_options.fields = ['name', 'subject', 'status']; } get_row_html(data) { - return `${data.subject}`; + const sign = data.status === 'Open' ? '' : '✔'; + return `

${sign} ${data.subject}

`; } } diff --git a/server/index.js b/server/index.js index 3a3ef7ca..f5852bdf 100644 --- a/server/index.js +++ b/server/index.js @@ -36,10 +36,6 @@ module.exports = { app.use(bodyParser.urlencoded({ extended: true })); app.use(express.static('./')); - app.use(function (err, req, res, next) { - console.error(err.stack); - res.status(500).send('Something broke!'); - }) // routes rest_api.setup(app); diff --git a/tests/test_document.js b/tests/test_document.js index c98ff300..2cfbec15 100644 --- a/tests/test_document.js +++ b/tests/test_document.js @@ -48,7 +48,7 @@ describe('Document', () => { () => { doc.set('status', 'Illegal'); }, - frappe.ValueError + frappe.errors.ValueError ); }); diff --git a/tests/test_rest_api.js b/tests/test_rest_api.js index e94ca366..f7aaa0f3 100644 --- a/tests/test_rest_api.js +++ b/tests/test_rest_api.js @@ -4,7 +4,7 @@ const fetch = require('node-fetch'); const helpers = require('./helpers'); const { spawn } = require('child_process'); const process = require('process'); -const Database = require('frappe-core/backends/rest_client').Database +const RESTClient = require('frappe-core/backends/rest_client') // create a copy of frappe @@ -19,7 +19,7 @@ describe('REST', () => { await frappe.init(); await frappe.login(); - frappe.db = await new Database({server: 'localhost:8000'}); + frappe.db = await new RESTClient({server: 'localhost:8000'}); frappe.fetch = fetch; // wait for server to start diff --git a/tests/test_router.js b/tests/test_router.js new file mode 100644 index 00000000..b2c32384 --- /dev/null +++ b/tests/test_router.js @@ -0,0 +1,28 @@ +const Router = require('frappe-core/common/router'); +const assert = require('assert'); + +describe('Router', () => { + it('router should sort static routes', () => { + let router = new Router(); + router.add('/a', 'x'); + router.add('/a/b', 'y'); + router.add('/a/b/clong/', 'z'); + + assert.equal(router.match('/a/b').handler, 'y'); + assert.equal(router.match('/a').handler, 'x'); + assert.equal(router.match('/a/b/clong/').handler, 'z'); + }); + + it('router should sort dynamic routes', () => { + let router = new Router(); + router.add('/edit/todo/mytest', 'static'); + router.add('/edit/:doctype/:name', 'all'); + router.add('/edit/todo/:name', 'todo'); + + assert.equal(router.match('/edit/todo/test').handler, 'todo'); + assert.equal(router.match('/edit/user/test').handler, 'all'); + assert.equal(router.match('/edit/todo/mytest').handler, 'static'); + }); + + +}); \ No newline at end of file diff --git a/utils/index.js b/utils/index.js index 90e17d69..ddb55f6b 100644 --- a/utils/index.js +++ b/utils/index.js @@ -5,7 +5,10 @@ module.exports = { async_handler(fn) { return (req, res, next) => Promise.resolve(fn(req, res, next)) - .catch(next); + .catch((err) => { + // handle error + res.status(err.status_code).send({ error: err.message }); + }); }, async sleep(seconds) {