Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(catalogue)!: convert catalogue ssr user interface to flat model. BREAKING CHANGE #4114

Merged
merged 44 commits into from
Aug 19, 2024

Conversation

mswertz
Copy link
Member

@mswertz mswertz commented Aug 6, 2024

closes https://github.com/molgenis/GCC/issues/592

breaks compatability with the previous unflat data model, therefore breaking change?

What are the main changes you did:

  • added demo data for flat model
  • replaced catalogue-demo with flat model
  • updated nuxt-ssr to work with the flat data model

how to test:

  • apply test plan

Remaining issues collection screen:

  • detail views for datasets, collection events, subpopulations
  • publications
  • marker paper
  • contributors
  • lead contributor
  • partner organisations
  • lead organisation
  • about page for a catalogue
  • relations to other collections (e.g. networks), grouped by type
  • datasets details is broken (e2e test FETALCRL_22112016)

todo:

  • deal with the new 'repeat' pattern
  • update menu to show listing of all types of resources
  • test the 'umcg' pattern of using the catalogue app still works
  • add all missing details to collection detail view (can be later)
  • update CatalogueSiteMap.java
  • update e2e tests

data migrations missing, tbd later:

  • designType
  • startDataCollection endDataCollection

proposed data model / ontology changes, tbd later:

  • 'Clinical trial' is not a type of cohort but type of collection
  • rename designType back to design or better merge with cohortType?
  • do we need collection type?
  • renaming peopleInvolved
  • can 'organisationalunit', 'biobank', 'catalogue' and 'network' be collection types and/or should we resort to superclass again?

later:

  • update test plan when all migrations finished
  • decide if we want to have per type specific screens (now 'cohorts' has become 'collections')
  • reconsider naming of some things ;-)
  • do we need qualified relation? I.e. role of relation with other collection?
  • a lot of hand work, try migrate to the metadata based viewer (what settings are missing?)
  • linkage options

@mswertz mswertz marked this pull request as draft August 6, 2024 12:30
@mswertz mswertz changed the title feat(catalogue)!: convert catalogue ssr user interface to flat model. BREAKING CHANGE> feat(catalogue)!: convert catalogue ssr user interface to flat model. BREAKING CHANGE Aug 6, 2024
apps/cat.env-sample Outdated Show resolved Hide resolved
Comment on lines +21 to +30
"Cohort study",
"cohorts",
"Study",
"studies",
"Network",
"networks",
"Databank",
"databanks",
"Data source",
"datasources");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the upper and lower case mix ?

@@ -31,7 +41,7 @@ defineProps<{
<span
class="hover:flex items-center justify-items-end align-middle min-w-[2rem] hover:z-50 py-2"
>
{{ columnProps.value.cohort.id }}
{{ columnProps.value }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

columnProps ? , no need for .value en template

@@ -1,15 +1,15 @@
<script setup lang="ts">
import type { ICohort, IVariableWithMappings } from "~/interfaces/types";
import type { ICollection, IVariableWithMappings } from "~/interfaces/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ICollection to use as a collection of type T ? maybe not relevant for catalogue UI

menu.push({
label: "Data sources",
link: `/${route.params.schema}/ssr-catalogue/${catalogueRouteParam}/datasources`,
label: "collections(" + props.catalogue.collections_agg?.count + ")",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not work for all paths, and maybe pushed to much information , ui needs to make constant tradeoffs between overview, consistency. Suggest to remove and reconsider in separate feature pr

<ContentBlock
v-if="projectCatalogues.length === 0 && thematicCatalogues.length === 0"
title="No Catalogues found"
description="Please add catalogues via admin user interface"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if we should to use the ui for these kind of data management instructions

Comment on lines +54 to +65
// add the search to the filters
filterBuilder = {
...filterBuilder,
...{ _or: [{ _search: searchFilter.search }] },
};
} else {
// add the search to the filters
filterBuilder = {
...filterBuilder,
...{ _search: searchFilter.search },
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge branch and move adding the 'or' down the tree ?

Comment on lines 13 to +14
* @param variables
* @param cohorts
* @param collections
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest to remove in favour of ts types

Schema cohortStaging = database.createSchema(COHORT_STAGING);
AvailableDataModels.DATA_CATALOGUE_COHORT_STAGING.install(cohortStaging, true);
assertEquals(19, cohortStaging.getTableNames().size());
}

@Disabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete ? , or add reason why it is disabled

@connoratrug
Copy link
Contributor

warnings during dev

`
WARN Components directory not found: /Users/[user]/Code/emx2/molgenis-emx2/apps/nuxt3-ssr/components/global/icons 10:36:29 AM

WARN Components directory not found: /Users/[user]/Code/emx2/molgenis-emx2/apps/nuxt3-ssr/components/viz `

Copy link

sonarcloud bot commented Aug 19, 2024

Copy link
Contributor

@connoratrug connoratrug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good and checked the basics, wait for e2e test before merge

@connoratrug connoratrug merged commit 01be7d4 into master Aug 19, 2024
4 of 5 checks passed
@connoratrug connoratrug deleted the feat/catalogue_flat_ui branch August 19, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants