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

Pull edit out #164

Merged
merged 5 commits into from Aug 17, 2015
Merged

Pull edit out #164

merged 5 commits into from Aug 17, 2015

Conversation

TakenPilot
Copy link
Contributor

This PR is to help us find bugs that are currently escaping notice by confining all bugs and defects to within a particular edit. This is done by preventing any corruption of the cache that is currently happening when people edit objects retrieved from the cache.

Architecture

  • edit.js and db.js have been moved to their own edit folder. Edit.js has become index.js, and should be referred to as require('./services/edit'). Ideally, db.js will never be referred to, but all actions will be made discrete inside edit.js and well tested. (hahaha! right? :p)
  • Saving to edit.js now only takes a single parameter data, since we always have _ref at the base to say where this object should be saved. Creating a new components still takes two parameters (uri, data).
  • Saving a mix of Amphora and ClayKiln style data is now banned. All data must be in proper ClayKiln style to save, which is {value, _schema}. A convenience function has been exposed called toClayKilnStyle that can help temporarily until we get everyone being consistent, and it's already marked as deprecated and also logs a warning when it is used. It's currently being used in the forms.js, which apparently hasn't changed in a very long time.
  • Saving any data not in the schema is now banned. It will throw an error.
  • Saving any data to components without schemas is now banned. It will throw an error, since if we don't know how to edit them (no schema), we shouldn't be trying to. They're essentially read-only.
  • Saving any data to the reserved field names is now banned. The reserved names so far are ['_ref', '_groups' '_self', '_components', '_pageRef', '_pageData', '_version', '_refs', 'layout', 'template']. We use those for special things.
  • A new cache.js has been added into the edit folder. All things that are cached for editing go there. All functions are cached in some way, and all functions return read-only values.
    • getData: Cached completely. Returns read-only value. Cleared on successful edit.
    • getDataOnly: Cached completely. Returns read-only value. Cleared on successful edit.
    • getSchema: Cached completely. Returns read-only value. Never cleared.
    • saveThrough: Save an object, save result to cache immediately (no second fetch). Clear other cache values on success. Returns read-only value.
    • createThrough: Create a new object, save result to cache immediately (no second fetch). Clear other cache values on success. Returns read-only value.
  • A new control.js has been added into the edit folder. All things that constrain objects go there. It has two functions:
    • setReadOnly: Make an object into something that cannot be modified.
    • memoizePromise: Same as _.memoize, but asynchronous. It's super well tested code that I use in my other projects, and is available from my public gist.

Bug Fixes

  • Schemas are now entirely read-only, and any attempt to edit them will error.
  • All caching is read-only, and any attempt to edit the cached copies will error.
  • edit.js getData and getDataOnly return clones of the cached values.
  • db.js uses HTML5 events for send function instead of 1998's onreadystatechange.
  • Deep-cloning any data with a _schema on an Array will lose the schema because we're doing something weird. This is now fixed when using edit.js, but if you clone the data yourself, you're going to have a bad time.

Note: This will break things on purpose to highlight our mistakes, so they can be corrected more easily. That said, I can't seem to find anything broken at the moment. I can edit tags, edit headlines, and edit paragraphs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 81.801% when pulling b28efa3 on pull-edit-out into f450842 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 81.827% when pulling 30770fd on pull-edit-out into f450842 on master.

@nelsonpecora
Copy link
Contributor

👍

nelsonpecora added a commit that referenced this pull request Aug 17, 2015
@nelsonpecora nelsonpecora merged commit e3b67ca into master Aug 17, 2015
@nelsonpecora nelsonpecora deleted the pull-edit-out branch August 17, 2015 16:12
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