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: Update directory app with services #4501

Merged
merged 46 commits into from
Dec 4, 2024

Conversation

connoratrug
Copy link
Contributor

@connoratrug connoratrug commented Nov 18, 2024

What are the main changes you did

Update the app to allow user to request services

functional:

  • allow user to filter biobanks by service type
  • allow add services to the request/cart
  • add selected services to negotiator request
  • show service details in service report

non-functional:

  • switch touched files to typescript
  • fix issues found by typing
  • switch touched files to script setup
  • pullout ui components from 'smart' components
  • use ui components and containers to have consistent style
  • remove unused code

How to test

  • explain here what to do to test this (or point to unit tests)

Checklist

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

Change Ontology.Capabilities.name and Ontology.Capabilities.label into [Schema].Services.name
@connoratrug connoratrug changed the title feat: update initial page query feat: Update directory app with services Nov 18, 2024
- add checkbox to service card
- move sections to bioback card folder
- add typing to checkout store
- setup stuff to add selected services
- remove unneeded imports
- make table html valid
pass params direct as typed value, instead of wrapper object
- disable tab is list if empty
@esthervanenckevort
Copy link
Member

Looking at the user interface changes I think this is 95% what we would like to see. The card with tabs for collections, services and organisation details is quite clean. Also good that you made the split between collections and services in the request cart.

Some improvements:

  • The services search facet must use the label instead of the name attribute to show the items.
  • The service types is quite a long list and the display could be improved by using the category to group the services in a tree. In the data model they all have a category, the category (Service Category ontology type) should be used as the top level of the tree.
  • The data that is passed to the Negotiator does not correctly fill in the 'humanReadable', it prepends the actions to the text "User actions taken: " It should append the text instead. See:
{
"url":"https://preview-emx2-pr-4501.dev.molgenis.org",
"humanReadable":"Biobank services: sample-collection, sample-data-management, sample-processing, sample-quality-control, sample-storage. User actions taken: ",
"resources":[{"id":"bbmri-eric:serviceID:DE_1234","name":"Biobank Service"}]}

@connoratrug
Copy link
Contributor Author

connoratrug commented Dec 2, 2024

  • The services search facet must use the label instead of the name attribute to show the items.

This a config thing in the current setup, i'm not sure on how to do this ( or is its possible ) , maybe someone else does i checked its in the config, i updated the initial config to use the label

  • The service types is quite a long list and the display could be improved by using the category to group the services in a tree. In the data model they all have a category, the category (Service Category ontology type) should be used as the top level of the tree.

Suggest to move this to a separate pr , as i'm not sure if this fits within the current filter model , again maybe someone else knows this ?

  • The data that is passed to the Negotiator does not correctly fill in the 'humanReadable', it prepends the actions to the text "User actions taken: " It should append the text instead. See:
{
"url":"https://preview-emx2-pr-4501.dev.molgenis.org",
"humanReadable":"Biobank services: sample-collection, sample-data-management, sample-processing, sample-quality-control, sample-storage. User actions taken: ",
"resources":[{"id":"bbmri-eric:serviceID:DE_1234","name":"Biobank Service"}]}

I do not think this pr changes this ( i'll check ), so if we want to change it a suggest a separate pr , to keep things moving

@connoratrug connoratrug marked this pull request as ready for review December 2, 2024 12:41
Copy link
Member

@esthervanenckevort esthervanenckevort left a comment

Choose a reason for hiding this comment

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

Tested functionality:

  • Filtering works & services are shown on the main page and can be added to the cart
    Few omissions:
  • Services are not shown on the biobank details page
  • Services details page does not have an add button
  • Service types in main page are not shown in hierarchy

@connoratrug
Copy link
Contributor Author

Tested functionality:

  • Filtering works & services are shown on the main page and can be added to the cart
    Few omissions:
  • Services are not shown on the biobank details page
    Yes, i agree, propose to merge and then work on this en the next item as this used the same code , but needs a little refactor and cleanup to avoid code duplication
  • Services details page does not have an add button
    Yes, i'm working on this , but due to needed refactor i propose to move it to its own pr
  • Service types in main page are not shown in hierarchy
    where , how to you mean in de filter ? on the cards , could you elaborate , create a ticket ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Group functions and computeds a bit more


const emit = defineEmits(["update:modelValue"]);

const errorMessage = ref(""); // New reactive error state
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comment


export async function applyBookmark(watchedQuery) {
export async function applyBookmark(watchedQuery: Record<string, string>) {
console.log("applying bookmark", watchedQuery);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove debug line


if (bookmark) {
checkoutValid.value = true;
// todo need to add service stuff to the bookmark
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Comment on lines 369 to 374
// todo: is this needed?
// organization: {
// id: biobank.value,
// externalId: biobank.id,
// name: biobank.label,
// },
Copy link
Contributor

@chinook25 chinook25 Dec 4, 2024

Choose a reason for hiding this comment

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

Expand comment to better explain why this is commented

Copy link
Contributor

Choose a reason for hiding this comment

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

Make issue for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the todo and i'll create an issue

value?: IFilterOption[] | string | boolean,
fromBookmark?: boolean
) {
console.log("updateFilter", unref(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

debug line

@@ -6,7 +6,7 @@ import monacoEditorPlugin from "vite-plugin-monaco-editor";

const HOST =
process.env.MOLGENIS_APPS_HOST || "https://bbmri-emx2-test.molgeniscloud.org";
const SCHEMA = process.env.MOLGENIS_APPS_SCHEMA || "ERIC";
const SCHEMA = process.env.MOLGENIS_APPS_SCHEMA || "directory";
Copy link
Contributor

Choose a reason for hiding this comment

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

undo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the ERIC schema is no longer the up to date schema , i like to keep the schema name 'directory' to make it clear this is the app dev 'demo' schema

Copy link

sonarqubecloud bot commented Dec 4, 2024

@connoratrug connoratrug merged commit 7aa45a0 into master Dec 4, 2024
7 checks passed
@connoratrug connoratrug deleted the feat/directory-service-model branch December 4, 2024 13:03
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.

3 participants