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

Search schema #58

Merged
merged 3 commits into from
Dec 18, 2015
Merged

Search schema #58

merged 3 commits into from
Dec 18, 2015

Conversation

paultannenbaum
Copy link
Contributor

Fixes issue #20

  • Moves search schema to be its own model, accessible through ED. Created appropriate relationships.
  • Adds search schema detail view
  • Adds search schema edit view
  • Adds search schema create view
  • Adds content editable component
  • Adds code-highlighter component

export default Ember.Controller.extend({
schemaName: '',
schemaContent: ''
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This holds the temporary values of the new schema until they are ready to be sent to the server.

@dmitrizagidulin
Copy link
Contributor

Tested, looks great overall. (Btw, fantastic job integrating highlight.js!)

Couple of feedback items:

  1. How do you feel about adding a border to the editing textbox, so that it looks like a textbox? This may be just me, but I was confused initially, like, wait, so am I actually editing? (cause it was the same grey background). And possibly change the background color to white, when editing?

  2. Can we make the 'create new schema' link be an action button?

  3. Does it make sense to change the link to the schema view the same style as our other links to gui resources? (like the link to the index view, to the bucket type, etc).

  4. Let's add a 'Download' (or 'View Raw') button on the schema detail screen, next to the Edit. Which links to the schema directly, so that people can download it to their desktop (to use in an external editor, etc)..

  5. Slight tweak to the Create Schema workflow. Right now, when you go to the New Schema screen, fill out the fields and press Create, it pops up an alert, schema created!, and remains on the edit screen. Can we instead redirect to the 'Show Schema' route/view?

  6. And speaking of the Show Schema view - if I directly type in the url to view a schema, some of them work, and some fail with a TypeError: model is undefined. So for example, this url works:

    http://127.0.0.1:9000/cluster/production1/schema/_yz_default
    

    But this one fails, redirects to an error route:

    http://127.0.0.1:9000/cluster/production1/schema/my_schema
    

    And has this on the console:

    TypeError: Cannot read property 'get' of undefined
    at afterModel (http://127.0.0.1:9000/assets/ember-riak-explorer.js:9382:27)
    application.js:9 
    

    I suspect this is because _yz_default is used by one of the indexes on the cluster, and my_schema is unused by any indexes. But, it is still a valid schema that exists in Riak.

    The Show and Edit Schema views should instead just request the schema from Riak, and if it comes back, load it up and use it. And if it gives a 404 Not Found, let's have an error message to that effect, "This schema was not found."

@paultannenbaum
Copy link
Contributor Author

@dmitrizagidulin Thank you for the excellent feedback. All the recommended changes have been incorporated. Points 5 and 6 were related, and originally I did the alert as I didn't know what to do with a schema that wasn't associated with an index (since that is how we are creating the list), but thinking about it after your feedback I realized the obvious fix of just creating one in the route if the cluster doesn't currently have that schema. So thanks, its a much cleaner flow now.

@dmitrizagidulin
Copy link
Contributor

Brilliant! Thanks, Paul! Looks great now, +1 here.

One other minor question - there's some kind of indentation on the first line of the schema xml, in view mode. (Doesn't seem to be there in edit mode). Like:

            <?xml version="1.0" encoding="UTF-8"?>
<!--
 Licensed to the Apache Software Foundation (ASF) under one or more

Intended, or, worth fixing?
(In either case, feel free to merge).

@paultannenbaum
Copy link
Contributor Author

Yeah, I noticed the indenting too, it is something to do with highlight.js. I spent about 30 minutes debugging what was going on and couldn't figure out what was causing the issue. I figured it was probably a rabbits hole and shelved it for now, but I agree that its not visually very pleasant. Will try and fix later when doing the UX overhaul.

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.

2 participants