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

500 error if units are not defined in mapping #2368

Closed RDmitchell closed 2 years ago

RDmitchell commented 4 years ago

Expected Behavior

I thought I could leave the units pulldown blank in mapping for the fields that have that pulldown

Actual Behavior

Leaving the units blank caused a 500 error

Steps to Reproduce

RDmitchell commented 4 years ago

See imported file plus PDF of the Sentry error in this folder G:\My Drive\SEED Shared Folder\Software Development\Issues\Issue 2368

Direct link to Sentry error PDF https://drive.google.com/file/d/1N3yTjs0ztm_fYliOWkRly29-IxJcjNKe/view?usp=sharing

adrian-lara commented 4 years ago

Thanks for catching this. This was addressed with this change.

For more background, this error happened because there's logic on the backend to deduplicate components of column mappings whenever a field is being mapped to columns with the same name. In most cases, we don't expect this to happen since, once a user maps to a column of name "Gross Floor Area", we won't create a new mapping. We'll just use an old/pre-existing mapping. But since, units were completely dropped from this mapping, SEED didn't recognize the old mapping as being the same and tried to create another. After a few steps including deleting the duplicate column mappings components, SEED tries to create a new column mapping and the error-causing code is hit. When you came back to the page and tried mapping again, the duplicates have been deleted at that point so SEED goes down a different logical flow, avoiding that bug.

adrian-lara commented 4 years ago

Worth also discussing - I'm not sure that this necessarily has anything to do with leaving empty units. Though I'd vote to disable the "Map Your Data" button when units are empty.

adrian-lara commented 4 years ago

This is an edge case where rerunning the same operation after the error results in successful operation. We won't be patching this.

We'll go ahead and disable the button when units are empty as a less urgent fix.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity within 60 days. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 2 years ago

This issue has been closed automatically. If this still affects you please re-open this issue with a comment or contact us so we can look into resolving it.