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

portal 4.0.0 functionality described in portal_config.md #1097

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

portal 4.0.0 functionality described in portal_config.md #1097

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c2f13bc
portal 4.0.0 functionality described in portal_config.md
aartivnkt Sep 20, 2022
3a960d2
Merge branch 'master' into feature/portal_4.0.0_readme_updates
May 26, 2023
f174d08
remove merge artifact, extra closing bracket
May 26, 2023
370fe75
adjusted comments so that something can be added
May 26, 2023
d3d36e9
Merge branch 'master' into feature/portal_4.0.0_readme_updates
ocshawn May 26, 2023
087942e
Merge branch 'master' into feature/portal_4.0.0_readme_updates
ocshawn Jun 21, 2023
bf64303
Merge branch 'master' into feature/portal_4.0.0_readme_updates
ocshawn Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion docs/portal_config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,27 @@ Below is an example, with inline comments describing what each JSON block config
"enabled": true
},
"authorization": {
"enabled": true // toggles whether Discovery page displays users' access to studies. If true, 'useArboristUI' must also be set to true.
"enabled": true, // toggles whether Discovery page displays users' access to studies. If true, 'useArboristUI' must also be set to true.
"columnTooltip": "Filter by data access", // new in data portal 4.0.0, this is to clarify availability versus accessibility using tooltip
"supportedValues": { // new in data portal 4.0.0: supportedValues configures how to show data accessibility/availability on the portal
"accessible": {
"enabled": true, // to enable showing "Available" on the discovery page
"menuText": "Available"
},
"unaccessible": {
"enabled": false, // to disable showing "Not Accessible" on the discovery page"
"menuText": "Not Accessible"
},
"pending": {
"enabled": true,
"menuText": "Pending" // to enable showing "Pending" on the discovery page
},
"notAvailable": {
"enabled": true,
"menuText": "Not Available"
}
}
}
},
"tagsColumn" : {
"enabled": true // toggles if tags should be rendered in a column
Expand All @@ -481,6 +501,23 @@ Below is an example, with inline comments describing what each JSON block config
"tagSelector": {
"title": "Associated tags organized by category"
},
"studies": [ // new in data-portal 4.0.0, this configures addition of tutorial nbs to specific studies. Shown here in BRH example. For HEAL, set to []
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you elaborate a bit more about how this canbe used? with those match and fieldsToValues?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, the level of detail you are requesting is easier to communicate via code than documentation (e.g. someone needing to know more detail than the example can look at the data portal code)

Copy link
Collaborator

@mfshao mfshao Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

yeah true but the code is lack of comment as well 🤦 And while we can improve documentation now, why wait?

we should try our best to not make information silos to devs who write the code

Copy link
Collaborator

@mfshao mfshao Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

if you think it will be too verbose to put in this file, you can write a dedicated markdown like https://github.com/uc-cdis/data-portal/blob/master/docs/ecosystem_browser.md

Copy link
Contributor

Choose a reason for hiding this comment

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

Code doesn't always need to have comments and usually should be self documenting. Excessive documentation makes maintenance more tedious.

And while we can improve documentation now, why wait?

Because the anticipation of a BRH go live has been going on for months and the team may finally have enough momentum for it to actually happen.

we should try our best to not make information silos to devs who write the code

Putting an emphasis on writing clean, self documenting code instead of excessive standalone documentation is actually a win-win for product managers and developers: encourages product managers to look at source code and obviously makes the job more fun for a developer.

Copy link
Collaborator

@mfshao mfshao Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Excessive documentation makes maintenance more tedious.

Agreed. But I don't think the ask is excessive. If a ppl needs to go back to the original PR or code base checks among 100+ lines of changes to figure out why my portal is crashing because of missing a [] in config file or understand why the matches working from a config file doesn't work for another one, how is that code "clean and self documenting"?

encourages product managers to look at source code

Good thinking. But you cannot expect all PMs have the same level of understanding of code as devs. This is just unrealistic. And PMs and Devs are not the only person who will update portal configs to deploy a new feature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey guys, I'll add some more documentation to this. May not be today, but definitely soon. I think we need a good balance of clean code and documentation, so I'll go over that and try to fill the gaps as needed. Portal is very new to me as well, so may be a good starting point to see if I can make it such that anyone can follow.

{
"match": {
"short_name": "Methodology and Advanced Analytics Resource Center"
},
"fieldsToValues": [
{
"data_access_method": "API, Manifest"
},
{
"tutorial_notebook": "YES"
},
{
"tutorial_notebook_link": "/dashboard/Public/notebooks/JCOIN_MOUD_accessibility_jupyter_notebook_BRH.html"
}
]
},
"studyColumns": [ // configures the columns of the table of studies.
{
"name": "Study Name",
Expand Down