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.

Sign up for GitHub

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

Jump to bottom

Add is_a directed search functionality to term search #34

Merged
merged 13 commits into from
Oct 29, 2020
Merged

Add is_a directed search functionality to term search #34

merged 13 commits into from
Oct 29, 2020

Conversation

Freymaurer
Copy link
Collaborator

@Freymaurer Freymaurer commented Oct 21, 2020 •
edited

Features added:

  • Add is_a directed search functionality to term search.
  • Restored bulma imports in style sheets for bulma-extensions switch and tooltip.
  • change addBuildingBlock function to add 3 columns with two cols hidden. These extra columns will hold additional ontology term info in the future
  • add new sql dump with is_a directed search procedure and is_a relationships in TermRelationship table

Do not merge until:

  • is_a directed search returns all children and not only direct children of is_a relationships.
  • insert term function also inserts the additional info in the new extra columns of addBuildingBlock.

@Freymaurer
Update to SAFE stack v2.2
9191c19 
- Update to only dotnetcore3.1 projects
- simplify paket groups
- [BUGFIX] Error with Fable solved by setting the version for Fable.SimpleJson to 3.11.0
- Migrate database from MSSQL to MySql
-> Add docker-compose.yml file for MySql+Adminer instance
-> Add db.sql dump file to initialize Swate DB to MySql docker instance
-> Adjust fake.fsx to automatically start docker-compose with "OfficeDebug" target
- Update README with information about docker.
@Freymaurer
Apply Requested Changes According to PR #32
20f4b5f 
- Return to previous fake-cli version 5.20.3
- Fix typo in README.md
- Return to inline dependencies in build.fsx
- Allow users to set MySql password during setup with optional paramater
- Remove Client side user secret
@Freymaurer
Replaced userfolder in webpack.config with placeholder
beb28dc
@Freymaurer
add reminder about placeholders to README.md
1ba2838
@Freymaurer
Bug-Fix:
3c3dadd 
- add Fable.Elmish to Client paket.references
- add testApi and increase specificity of api forwarding
- add install.cmd and related info to README.md
- revert build.fsx inline dependencies to paket build-group
@Freymaurer
Add is_a relationship directed search option for terms
6c35b30
@Freymaurer
Reactivated imports in style sheet to regain support for bulma switch…
976bc2e 
… and tooltip
@Freymaurer
replace inserts with placeholders for fake.build -t setup
11c8eb8
@Freymaurer
change addBuildingBlock to add three columns with two hidden for onto…
313bd85 
…logy id's
@Freymaurer
Merge new changes with previous PR commit
d6d2dd6
@Freymaurer
Add new SqlDump and restore placeholders
3a30400
@Freymaurer Freymaurer mentioned this pull request Oct 21, 2020
Closed
@Freymaurer
- modify is_a directed search to recursively iterate through all chil…
df83030 
…dren and not only the direct ones.

- modify insert term to also insert "Term Source REF" and "Term Accession Number" when using a selected Term
@Freymaurer
- update mysql dump with improved stored procedure for is_a directed …
5cfb58e 
…search.

- added DevelopmentConnectionString.fs to project in LAST COMMIT. Now docker-compose.yml, webpack.config.js and DevelopmentConnectionString.fs can be ignored during commiting.
kMutagene
kMutagene approved these changes Oct 29, 2020
View reviewed changes
Copy link
Member

@kMutagene kMutagene left a comment

Choose a reason for hiding this comment

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

Good stuff. Please check my comments, but this can be merged as is.

src/Client/Views/BaseView.fs
@@ -27,6 +27,7 @@ let createNavigationTab (pageLink: Routing.Page) (model:Model) (dispatch:Msg-> u
]
] [
Text.span [] [
/// does not work for me in excel, and in excel online i have a fixed width that always shows the icon
Copy link
Member

Choose a reason for hiding this comment

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

Please open an issue if this does not work anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i noticed it updates everytime i switch the subpage, but we should switch to css:

@media only screen and (max-width: XXXpx) { previousClass: {style} }

Copy link
Member

Choose a reason for hiding this comment

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

Yes that is way cleaner. Can you open an issue for that so we have it in the backlog?

src/Server/Server.fs
@@ -94,10 +108,25 @@ let testWebApp =
|> Remoting.buildHttpHandler


let annotatorDocs = Docs.createFor<IAnnotatorAPI>()
Copy link
Member

Choose a reason for hiding this comment

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

Does this work well? if so, we should expand it and provide an api doc link in the repo and on the page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This works really well if you use examples. Otherwise the documentation lacks in comparison to OpenApi plus you cannot choose which Apis to document. The documentation for all api is automatic and you only need to fill in description and examples.

@kMutagene kMutagene merged commit 62d07f2 into nfdi4plants:master Oct 29, 2020
@Freymaurer Freymaurer mentioned this pull request Nov 2, 2020
Closed
2 tasks
@kMutagene kMutagene mentioned this pull request Nov 5, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kMutagene kMutagene kMutagene approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Freymaurer @kMutagene