Query action

[KeithJH]

Addresses issue #160 and should finish #183

[seanblue]

@KeithJH Trying to execute a QueryAction locks up the browser. See https://www.cs.drexel.edu/~kjh65/WAVED/query-action/?project=sean

[seanblue]

@KeithJH Creating and editing seems to work fine. Saving and loading are also good.

[seanblue]

@KeithJH I confirmed that my previous example is working now. Is the arguments feature supposed to work yet?

EDIT: I can confirm that the correct state is being selected, but the query finds no results. See https://www.cs.drexel.edu/~kjh65/WAVED/query-action/?project=sean2

EDIT2: The reason it's not working seems to be because states.json uses the full name for {{state}} and the test file I had uses the abbreviation. I think you're idea of adding the abbreviations to states.json (maybe under "abbrev" column) may be for the best.

[KeithJH]

@seanblue I've done limited testing, but I got the arguments thing to work far enough that clicking on a state hides it's glyphs (assuming correct data). You should be able to give it a test and let me know if anything doesn't work as expected.

EDIT: Just noticed your edits, it's a shame it doesn't send an email for it

[seanblue]

@KeithJH The reason it's not working for me seems to be because states.json uses the full name for {{state}} and the test file I had uses the abbreviation. I think your idea of adding the abbreviations to states.json (maybe under "abbrev" column) may be for the best.

[seanblue]

@KeithJH When I click a state, it gives the information Object {state: "Washington", x: 13.6, y: 21.16}. Do you know if it's possible to add the abbreviation in there? I ask because this format doesn't match states.json as I expected.

[seanblue]

@KeithJH Yeah it's annoying that edits aren't emailed. I didn't see yours until now either. I'm just going to comment normally from now on.

[KeithJH]

@seanblue That's where I want to put the abbreviation. states.json seems to be a good place to persist the data, I'll just have to grab it just like how the name is grabbed (it doesn't automatically grab fields). Do we have a states.json with abbreviations (did you create one, or would there be one wherever we got the original), or should I make one?

[seanblue]

I don't have one so you should just update the current one.

On May 17, 2014, at 7:18 PM, KeithJH notifications@github.com wrote:

@seanblue That's where I want to put the abbreviation. states.json seems to be a good place to persist the data, I'll just have to grab it just like how the name is grabbed (it doesn't automatically grab fields). Do we have a states.json with abbreviations (did you create one, or would there be one wherever we got the original), or should I make one?

— Reply to this email directly or view it on GitHub.

[KeithJH]

@seanblue Abbreviations should work with {{stateAbbreviation}}, though I'm up for a better name that's still descriptive enough. You have to create a new project so that states.json is copied from the updated source. Exampe: https://www.cs.drexel.edu/~kjh65/WAVED/query-action/?project=abbr

[seanblue]

Yeah, it's a bit wordy. But I understand that "abbrev" isn't very descriptive. I'm still partial to that because if it's documented they'll know and it's much shorter.

On May 17, 2014, at 8:34 PM, KeithJH notifications@github.com wrote:

@seanblue Abbreviations should work with {{stateAbbreviation}}, though I'm up for a better name that's still descriptive enough. You have to create a new project so that states.json is copied from the updated source. Exampe: https://www.cs.drexel.edu/~kjh65/WAVED/query-action/?project=abbr

— Reply to this email directly or view it on GitHub.

[seanblue]

@KeithJH Amazing, this will get exactly the demo I wanted! https://www.cs.drexel.edu/~kjh65/WAVED/query-action/?project=sean3

[seanblue]

@KeithJH I just reviewed the code. Pretty good, just minor cleanup needed. Shouldn't take too long I think. For the code style, nothing stood out, but please open it in eclipse to see if any styling issues exist.

[seanblue]

@KeithJH Still need the dependency checking https://github.com/KSHSK/WAVED/issues/183

[KeithJH]

@seanblue I won't be getting to address any of these issues before the presentation, unfortunately. I'd rather not rush things and break everything. You might just want to merge this into stable for the presentation.

On Sun, May 18, 2014 at 5:16 PM, seanblue notifications@github.com wrote:

@KeithJH https://github.com/KeithJH Still need the dependency checking

183 https://github.com/KSHSK/WAVED/issues/183

— Reply to this email directly or view it on GitHubhttps://github.com/KSHSK/WAVED/pull/271#issuecomment-43451878 .

[seanblue]

@KeithJH Please disable the dropdown to select Property or Query Action when editing.

[seanblue]

@KeithJH When do you plan to address the remaining issues?

[KeithJH]

@seanblue I should be able to get to them by the end of tomorrow.

On Tue, May 20, 2014 at 7:07 PM, seanblue notifications@github.com wrote:

@KeithJH https://github.com/KeithJH When do you plan to address the remaining issues?

— Reply to this email directly or view it on GitHubhttps://github.com/KSHSK/WAVED/pull/271#issuecomment-43694739 .

[KeithJH]

@seanblue I think I got to everything we talked about.

[seanblue]

@KeithJH Just looked through the commits and it looks good. Obviously a lot of changes were made, so I'll test everything from scratch once I get home from class.

[seanblue]

@KeithJH The deleting data subset error is being reported as an info. Please change that to use warning.

[seanblue]

@KeithJH I can't edit a Property Action for USMap. The issue is that the hidden ListProperty has an error somehow. The weird thing is I thought I fixed (worked around) this by skipping properties that didn't define displayTemplateName since they wouldn't be in the dialog. Adding that should fix the problem.

[seanblue]

@KeithJH Everything else looks good.

@kristiancalhoun @stnguyen09 @hpinkos This needs a second review.

[KeithJH]

@seanblue Repro steps for issue with the USMap? It seems to work fine to me at first glance. Is this currently an issue in master as well, or is the issue reintroduced in my branch?

[seanblue]

@KeithJH I can't get it to reproduce consistently, but since this is the second time I saw it, it's definitely something. If I find steps I'll let you know.

[seanblue]

@KeithJH It seems to have something to do with having glyphs (which relates to the error being on ListProperty) and clicking refresh. Full steps pending...

[seanblue]

@KeithJH I fixed the bug in master: https://github.com/KSHSK/WAVED/commit/657ba7e3645502a5b5c8809fc99cdd69386ca9ec

[seanblue]

@KeithJH Confirmed that the error message is correct now. If you don't mind, please update "The project must be saved and reloaded to reuse this filename." (in UploadData) and the ones in DataSubsetHelper to be warning. Thanks!

[seanblue]

@KeithJH Please open this in eclipse and fix some warnings. There is only one that baffles me that might not need to changed, in ActionHelper.

[KeithJH]

@seanblue I think I've addressed everything you've brought up.

[seanblue]

@KeithJH Looks good. I can't believe I didn't realize the one warning was because commas were used instead of semicolons. I'm signing off.

@hpinkos @stnguyen09 Waiting for second review.

[seanblue]

@kristiancalhoun Did you do a full review of this or just skimmed?

[KeithJH]

@hpinkos @stnguyen09 @kristiancalhoun Anyone want to do a second review so we can actually get this in master?

[stnguyen09]

I'll review it this afternoon

Steve Nguyen Drexel University - Computer Science st.nguyen09@gmail.com (484) 602-5138 On May 29, 2014 1:12 PM, "KeithJH" notifications@github.com wrote:

@hpinkos https://github.com/hpinkos @stnguyen09https://github.com/stnguyen09 @kristiancalhoun https://github.com/kristiancalhoun Anyone want to do a second review so we can actually get this in master?

— Reply to this email directly or view it on GitHubhttps://github.com/KSHSK/WAVED/pull/271#issuecomment-44557256 .

[stnguyen09]

Functionality looks good. Just a few comments

• Creating a data subset (and subsequently, editing/adding a query action) allows for an empty value field. It should be required and disable the save button.
• Editing a query action and removing the name of the action causes an error to come up like expected, but the save button isn't visually disabled (it is disabled, but the styling isn't applied).
• This merge with the coloring actions branch will be the worst thing ever

[seanblue]

@stnguyen09 I disagree with your first point. An empty value is perfectly valid. Maybe you want to filter blank values out of your data file. Maybe you're just setting up a Data Subset and plan to modify the field values with a Query Action.

[stnguyen09]

I disagree with your first point. An empty value is perfectly valid. Maybe you want to filter blank values out of your data file. Maybe you're just setting up a Data Subset and plan to modify the field values with a Query Action.

Fair point, I retract my comment

[seanblue]

@KeithJH You need to include the edit changes for EventHelper as well. Check for any other edit dialogs that need this change.

[KeithJH]

@stnguyen09 @seanblue Included edit changes for ko disable and looked for other areas that needed it. Besides glyphs (which we decided wasn't set up in an easy fashion to get this working) there was also Google Analytics. It also wasn't perfect but my attempt is on another branch (in case we don't use it): https://www.cs.drexel.edu/~kjh65/WAVED/disable-ga-add/?project=kjh65.

Repro to sub-par handling:

• Change UA to correct value
• Change category to correct value
• Change UA to incorrect value

The add button remains enabled until UA losses focus, so you can technically still click the add button before it gets disabled.

[seanblue]

I don't think disabling the GA add button is necessary.

On May 31, 2014, at 2:30 PM, KeithJH notifications@github.com wrote:

@stnguyen09 @seanblue Included edit changes for ko disable and looked for other areas that needed it. Besides glyphs (which we decided wasn't set up in an easy fashion to get this working) there was also Google Analytics. It also wasn't perfect but my attempt is on another branch (in case we don't use it): https://www.cs.drexel.edu/~kjh65/WAVED/disable-ga-add/?project=kjh65.

Repro to sub-par handling:

Change UA to correct value Change category to correct value Change UA to incorrect value The add button remains enabled until UA losses focus, so you can technically still click the add button before it gets disabled.

— Reply to this email directly or view it on GitHub.

[stnguyen09]

Just looked at the GA thing and agree that it isn't necessary. Once they hit the Add button the error shows up.

Otherwise I just did a quick functionality test again and everything looks good. Merging.