e-mission / op-admin-dashboard

An admin/deployer dashboard for the NREL OpenPATH platform
0 stars 9 forks source link

Updated readme and added demographics table #63

Closed achasmita closed 1 year ago

shankari commented 1 year ago

@achasmita

Code looks fine. Please indicate "Testing done" before I can merge. "If it is not tested, it is broken"

achasmita commented 1 year ago

@achasmita

Code looks fine. Please indicate "Testing done" before I can merge. "If it is not tested, it is broken" Screen Shot 2023-09-12 at 11 15 57 AMYes, testing is done and table looks fine.

shankari commented 1 year ago

@achasmita thank you for including the screenshot. Please do that for all PRs going forward.

However, the testing is not sufficient because it doesn't actually show any of the demographic survey fields. How do I know if the demographic survey is actually being parsed properly?

Also, please automatically exclude the vast majority of the metadata fields. We don't need the metadata.key (which is the same for all entries, or the metadata.type, ...

Please updated with more descriptive screenshots that show what you are actually testing.

achasmita commented 1 year ago

@achasmita thank you for including the screenshot. Please do that for all PRs going forward.

However, the testing is not sufficient because it doesn't actually show any of the demographic survey fields. How do I know if the demographic survey is actually being parsed properly?

Also, please automatically exclude the vast majority of the metadata fields. We don't need the metadata.key (which is the same for all entries, or the metadata.type, ...

Please updated with more descriptive screenshots that show what you are actually testing.

Here is the one after removing metadata.key and including demographic survey fields.Screen Shot 2023-09-12 at 3 32 45 PM

shankari commented 1 year ago

Does this only remove metadata.key? Please note my comment

Also, please automatically exclude the vast majority of the metadata fields.

You can, and should, include multiple screenshots to showcase the breadth of your testing.

achasmita commented 1 year ago

Does this only remove metadata.key? Please note my comment

Also, please automatically exclude the vast majority of the metadata fields.

i mean all the columns that starts with metadata.

achasmita commented 1 year ago

Here is the screenshot of all the columns it is displaying now:

Screen Shot 2023-09-12 at 3 41 06 PM Screen Shot 2023-09-12 at 3 42 57 PM Screen Shot 2023-09-12 at 3 43 22 PM

Screen Shot 2023-09-12 at 3 42 31 PM Screen Shot 2023-09-12 at 3 43 36 PM

Screen Shot 2023-09-12 at 3 42 06 PM

shankari commented 1 year ago

Please push these changes and I can merge

achasmita commented 1 year ago

new screenshots 
![Screen Shot 2023-09-12 at 4 26 11 PM](https://github.com/e-mission/op-admin-dashboard/assets/79387860/1094b34a-2d4d-4e20-a4b9-20ea2532f182)
Screen Shot 2023-09-12 at 4 25 50 PM