search

k-int / gokb-phase1

Original GOKb repo - Moving to https://github.com/openlibraryenvironment/gokb
http://www.gokb.org
Other
11 stars 5 forks source link

Text displayed after capturing a cell level edit is not valid #435

Closed kristenwilson closed 9 years ago

kristenwilson commented 9 years ago

After you capture a cell level edit, OpenRefine displays a pop-up window with code related to the edit. However, the text that's displayed in the pop-up is not valid JSON. Here's a sample:

if ( and (if( isNotNull(gokbCaseInsensitiveCellLookup('PublicationTitle')), and (isNotNull(cells[gokbCaseInsensitiveCellLookup('PublicationTitle')]), cells[gokbCaseInsensitiveCellLookup('PublicationTitle')].value == 'Coevolution'), false), if( isNotNull(gokbCaseInsensitiveCellLookup('title.identifier.eissn')), and (isNotNull(cells[gokbCaseInsensitiveCellLookup('title.identifier.eissn')]), cells[gokbCaseInsensitiveCellLookup('title.identifier.eissn')].value == '2325-6214'), false)), '2325-6213', value )

If I copy this text and try to reuse it, it will not work. I can get the correct code from the "Extract" dialogue on the undo/redo tab. However, since this pop-up comes up right on the main screen, I expect there will be an assumption that it can be copied and reused.

I would suggest replacing this with some eye-readable text similar to what Refine does natively with the other transformations. If that's not possible, then we should at least make it usable code.

ostephens commented 9 years ago

To note that the code displayed is valid GREL and can be pasted into a Cell Transformation window. Not sure if this is more or less intuitive than having to apply the code via JSON (i.e. open the Apply dialogue and paste in)

kristenwilson commented 9 years ago

When I pasted it into the Apply dialogue, I got an error saying "The JSON you pasted is not valid" and the transformation did not work.

kristenwilson commented 9 years ago

Oh, nevermind, I see what you're saying. That I think we're all so used to using the apply dialogue for macros that there's an assumption that's what you should you. Either way, it's confusing.

ostephens commented 9 years ago

I think I agree some eye-readable text would be better - there is no reason anyone should really cut and paste from this dialogue.

sosguthorpe commented 9 years ago

I must admit I was confused at first about this issue. The alert shouldn't be there and was a result of merging debug code into the extension so for that I apologise. It does flag the need for a valid alert though so I will use this issue as the placeholder for that. As this is basically a refine core function, I will need to add a custom Class for the cell-level edits so we can provide a nicer message.

sosguthorpe commented 9 years ago

This should be fixed as of the latest release.

kristenwilson commented 9 years ago

This will actually need to be retested after #408 is reinstated. Currently Refine is now allowing capture of cell level edits so there is nothing to test.

jhsolomon commented 9 years ago

Fix confirmed