SEED-platform / seed

Standard Energy Efficiency Data (SEED) Platform™ is a web-based application that helps organizations easily manage data on the energy performance of large groups of buildings.
Other
107 stars 55 forks source link

Handle errors when filtering columns with special characters #4511

Closed perryr16 closed 5 months ago

perryr16 commented 5 months ago

Any background context you want to provide?

A user reported an error when filtering on column "Annual District Chilled Water (kBtu)" at the /properties/filter endpoint Screenshot 2024-01-29 at 11 22 33 AM

ValueError: Column aliases cannot contain whitespace characters, quotation marks, semicolons, or SQL comments.
(12 additional frame(s) were not displayed)
...
  File "seed/utils/api.py", line 146, in _wrapped
    return fn(self, request, *args, **kwargs)
  File "seed/decorators.py", line 148, in wrapper
    response = func(self, request, *args, **kwargs)
  File "seed/lib/superperms/orgs/decorators.py", line 206, in _wrapped
    return _validate_permissions(perm_name, request, requires_org) or fn(self, request, *args, **kwargs)
  File "seed/views/v3/properties.py", line 522, in filter
    return get_filtered_results(request, 'property', profile_id=profile_id)
  File "seed/utils/inventory_filter.py", line 149, in get_filtered_results
    views_list = views_list.annotate(**annotations).filter(filters).order_by(*order_by)

What's this PR do?

Catches the error and sends the error back to the user via toast notification.

I was unable to replicate the exact error but did find another uncaught error scenario. I tested with the following columns

How should this be manually tested?

filter on the above columns

What are the relevant tickets?

Screenshots (if appropriate)

Screenshot 2024-01-29 at 11 55 54 AM Screenshot 2024-01-29 at 11 57 58 AM

perryr16 commented 5 months ago

Update

I tested with , (, [, {, |, and %. Only bracket [ and percent % (used in url encoding) caused errors. Replacing them with underscores _ resolved the errors.

Note that braces { didnt show up as they were interpreted as string interpolation.

Screenshot 2024-01-30 at 3 58 37 PM

RDmitchell commented 5 months ago

@kflemin -- I don't really know how to test this, and maybe I don't need to, but I did try filtering on the special characters that I imported for the international character issue, and the filter worked. image

RDmitchell commented 5 months ago

@kflemin -- I am going to set this to Done, but if you think it needs more testing, and it's anything I can do, let me know.

perryr16 commented 5 months ago

@RDmitchell -- This pr fixes filtering on a column when the header has special characters, not when the filter has special characters. Try filtering on a column that has a header with brackets [ ]

RDmitchell commented 5 months ago

@perryr16 -- a few questions.

Here is my sample spreadsheet with special characters in the header image When I import the file into SEED, this is what it looks like in the Mapping screen image But in the Mapping Review screen, the field that should be {Year Built} is displayed as ($ col.displayName translate $) image

This seems wrong ??

RDmitchell commented 5 months ago

@perryr16

And when I save the mapping and the data gets added to the inventory, the fields with % and [ are there, but not the curly bracket that was around Year Built, ie, {Year Built} image

If, from the Actions menu, I select "Show all populated fields" then I can see the Year Built field, shown as it was in the Mapping Review screen image

But I don't see it in that form in the Column List Profile -- I see "Year Built" but I think that is from a previous import.

Maybe the issue is that there was already a "Year Built" field, and then I added another one with curly brackets but the same name, which might have been confusing to deal with. image

RDmitchell commented 5 months ago

@perryr16 -- I should probably try a simpler example, where the field didn't already exist, and at least test that case.

Then maybe what I was doing, ie, a field with special characters around it that also existed without the special characters, should be a new issue?

perryr16 commented 5 months ago

Curly braces { } are a unique example as they are reserved operators in angular used to pass variables from the javascript file to the html file that the user sees. For example we might have a variable defined as user_name = 'demo@example.com' and when it is called to display on in the HTML it is coded as {$ user_name $} but angular knows to display it as demo@example.com.

So when you added braces to year built it confused angular as it thought it was looking for a variable that was not defined. It's possible to implement but more challenging

RDmitchell commented 5 months ago

@perryr16 -- would it be possible to do an error check on special characters like curly braces at mapping time, ie, let the user know that they can't use those characters, so that they can map to a field without them (hopefully that fixes the problem, ie, they can make the SEED header be the name without the curly braces?)

I don't think the program needs to deal with those characters, but it would be nice to warn the user that they shouldn't use them.

thx !!

perryr16 commented 5 months ago

@RDmitchell The simplest solution is to reject the import file if contains braces and have the user reupload with a modified file

RDmitchell commented 5 months ago

@perryr16 -- that seems fine to me. We just need to let the user know why it's being rejected with some sort of error message. Thanks !!