Open-Telecoms-Data / cove-ofds

https://ofds.cove.opendataservices.coop/
Other
1 stars 0 forks source link

Additional checks interface (add quotes to string values, make result table columns consistent) #13

Open duncandewhurst opened 1 year ago

duncandewhurst commented 1 year ago

Building on the check documentation in https://github.com/Open-Telecoms-Data/lib-cove-ofds/issues/8#issuecomment-1299492010, I've been thinking about how we display additional check results to users and have sketched up the following interface (colours, fonts, sizes are all placeholders, but hopefully it gives an idea):

image

My thinking is:

@odscjames is it feasible to implement something like this as part of the current sprint?

@lgs85 please take a look and share your feedback/suggestions (I'm definitely not a UI expert!)

odscjames commented 1 year ago

I've put on board and we'll see how much we can do - even just grouping them would be good.

odscjames commented 1 year ago

A first go at grouping is now in - as each type now has separate tables, this also allows someone to start checking which columns come up for each type of error and to specify which columns should come up. Try uploading the files in https://github.com/Open-Telecoms-Data/lib-cove-ofds/tree/8db98fd56cf2a5fe8a2563160c27ec3209bbba04/tests/fixtures/0_1_0_alpha

duncandewhurst commented 1 year ago

Thanks! I like the approach of having separate tables for checks that can relate to multiple objects. I think that's better than the 'Object identifier' approach in the issue description:

image

I've specified the columns that should appear in the results table for each check type and the values for each column in the additional checks spreadsheet.

I've also noted some things that need updating:

image

I've opened a couple of other issues related to the results interface:

duncandewhurst commented 1 year ago

Please can you add the following introductory sentence to the additional checks box:

Your data failed the following additional checks. You should check for errors in your mapping and data pipeline.

https://github.com/Open-Telecoms-Data/cove-ofds/commit/bf7f16789600626720f2991c13147a10dedbd914: please can you include the link to the schema docs as part of the feedback description paragraph, e.g. for the Node location type check:

Your data contains nodes with incorrect location types. /nodes/location/type must be 'Point'. For more information, see Geometry.type.

I've updated the feedback description column in the additional checks spreadsheet with the exact text to use for each check.

odscjames commented 1 year ago

please can you include the link to the schema docs as part of the feedback description paragraph Add the number of failures to the check heading (see image in issue description) Colour the additional checks box according to the failure severity in the additional checks spreadsheet.

all on 2022-11-08

Make the check title (e.g. 'Span route geometry' in the image above) either the same font size or smaller than the 'Additional checks' heading,

Getting this exactly right really depends on some good design work that should really be done for the whole site - for now I've just changed h3's to h4's and it kinda works. on 2022-11-08

duncandewhurst commented 1 year ago

please can you include the link to the schema docs as part of the feedback description paragraph Add the number of failures to the check heading (see image in issue description)

Looks good on 2022-11-08

Colour the additional checks box according to the failure severity in the additional checks spreadsheet.

Can't test due to https://github.com/Open-Telecoms-Data/lib-cove-ofds/issues/5#issuecomment-1307961555

Make the check title (e.g. 'Span route geometry' in the image above) either the same font size or smaller than the 'Additional checks' heading,

Getting this exactly right really depends on some good design work that should really be done for the whole site - for now I've just changed h3's to h4's and it kinda works. on 2022-11-08

Good enough for now, but agree that design work is needed outside of this sprint.

duncandewhurst commented 1 year ago

The priority outstanding action for this issue is adding the necessary columns to the results tables for each check, from https://github.com/Open-Telecoms-Data/cove-ofds/issues/13#issuecomment-1302762080:

I've specified the columns that should appear in the results table for each check type and the values for each column in the additional checks spreadsheet.

odscjames commented 1 year ago

Next version of library will have everything but path - that's a harder one to add. EDIT this is now on live!

(Whoops, I have phase ref name and organsation ref name but I've missed phase name and organsation name - I'll come back to those)

odscjames commented 1 year ago

Are the columns in the correct order?

eg span_start_node_not_found says "Network identifier (.id) / Span identifier (Span.id) / Path (e.g. /spans/0/start) / Node reference (Span.start | Span.end)" and that's the order the 4 columns should be in?

I'm not going to re-order them today, but it's just HTML tables in cove_ofds/templates/cove_ofds/additional_checks_table.html so this is an easy task for me or someone to pick up later.

duncandewhurst commented 1 year ago

Looking good so far!

Are the columns in the correct order?

Yep, please order the columns as specified in the spreadsheet.

odscjames commented 1 year ago

Columns are in correct order and all columns specified in spreadsheet should now be on live - please test!

duncandewhurst commented 1 year ago

Looks good, thanks!

A couple of outstanding tasks from earlier comments:

And some minor changes for consistency:

odscjames commented 1 year ago

Ticking backticks off as that already has a separate issue: https://github.com/Open-Telecoms-Data/cove-ofds/issues/35

odscjames commented 1 year ago

Ticking off show/hide comment, as that is moved to https://github.com/Open-Telecoms-Data/cove-ofds/issues/65

odscjames commented 1 year ago

All the label changes are ready for testing on live

duncandewhurst commented 1 year ago

The changes look good. Still need to add quotes to string values.

Also, whilst testing I noticed that the columns for the result tables for the organisation references check are inconsistent:

image

Please can you remove the 'field' column from the results tables for nodes and spans?

Edit: I've renamed the issue to reflect the outstanding tasks.

odscjames commented 1 year ago

Please can you remove the 'field' column from the results tables for nodes and spans?

Ready for testing on live

duncandewhurst commented 1 year ago

Looks good. Thanks!