210 - Define methods to format the API response in useDatasetAction

[nozomione]

Issue Number

Closing #210

Context reference: https://github.com/AlexsLemonade/refinebio-web/pull/192#discussion_r1350710760

NOTE: This PR adds new helper methods for formatting API response and is independently reviewed, since it's based on dev and not a feature branch.

Purpose/Implementation Notes

Implemented the methods to format the API response to the required data structure in the useDatasetAction hook and use them to pass the data prop value to the DatasetActionButton component.

Please see the latest UI here.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• [x] Lint and unit tests pass locally with my changes
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] I have added necessary documentation (if appropriate)
• [ ] Any dependent changes have been merged and published in downstream modules

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
refinebio-web	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 26, 2023 3:00pm

[nozomione]

@davidsmejia I applied your feedback and now those methods are in /helpers and are used in components (instead of initializing the useDatasetAction hook) - ready for another look, thank you, David.

[davidsmejia]

@davidsmejia I applied your feedback and now those methods are in /helpers and are used in components (instead of initializing the useDatasetAction hook) - ready for another look, thank you, David.

I'm sorry but I didn't mean outside of the useDatasetAction file, please keep it in the same file but define it so that it can be exported separately. Like in the example I provided. The reason for the change is to have co-locality of this formatting function because it is integral to making this hook work. Outside of the hook function because we do not need to call the hook function in order to get these data structures.

Let me know if what I am asking for is clear. Sorry again for not being more specific the first time.

[nozomione]

Outside of the hook function because we do not need to call the hook function in order to get these data structures.

@davidsmejia No worries! Actually, the getFormattedExperiment is now also used to pass the data prop (the same data structure) to the SamplesTable component from [experiment] (1250a984eb992364bc139d72ac6505a56301b08f) so it turned out to be good 👍

Those helpers are useful since that data structure (with the key word all - when passing to API, it's [ALL]) is commonly used in our codebase.

[nozomione]

Please move getFormattedExperiments and getFormattedExperiments list to the file useDaasetAction.js

Expected useDatasetAction.js

export const getFormattedExperiments = () => {}
export const getFormattedExperimentsList = () => {}
export const useDatasetAction = () => {}

We want to limit the helpers to just general helpers, (ie working with arrays or objects) but because these functions are integral to the useDatasetAction hook, I would like them located in the same file. By exporting getFormattedExperiment from the hook we can tell from the import that the function is used for that hook. Also, we can see in the same file how these data structures are being created and used.

Please let's keep them as shared helpers since is shared in the codebase (to format the API response) and is not exclusive to the useDatasetAction hook.

Please see this commit made in this PR 1250a984eb992364bc139d72ac6505a56301b08f.

Also I'm afraid that your suggestion (as below) is perhaps an anti-pattern in react, since hooks are meant to be initialized when they're used:

import { getFormattedExperiment } from 'hooks/useDatasetAction'

Instead, we should use all hooks as below (currently we follow this pattern throughout our codebase):

// Step 1: import the hook you want to use in a component
import { useDatasetAction } from 'hooks/useDatasetAction'

// Step 2: destruct the method you wish to use from the hook by invoking the hook
const { getFormattedExperiment }  = useDatasetAction()

Please let us keep the methods as helpers or follow the hook pattern to use them.

[davidsmejia]

Please see this commit made in this PR 1250a98.

Also I'm afraid that your suggestion (as below) is perhaps an anti-pattern in react, since hooks are meant to be initialized when they're used:

import { getFormattedExperiment } from 'hooks/useDatasetAction'

Instead, we should use all hooks as below (currently we follow this pattern throughout our codebase):

// Step 1: import the hook you want to use in a component
import { useDatasetAction } from 'hooks/useDatasetAction'

// Step 2: destruct the method you wish to use from the hook by invoking the hook
const { getFormattedExperiment }  = useDatasetAction()

Please let us keep the methods as helpers or follow the hook pattern to use them.

Hmm, I'm not sure I agree about the antipattern here. Specifically not sure what "since hooks are meant to be initialized when they're used" refers to here. We would continue to use the hook as we do only we would keep methods specific to preparing data for these hooks inside the same file.

useDatasetActions.js

export const getFormattedExperiments = () => {}
export const getFormattedExperimentsList = () => {}
export const useDatasetAction = () => {}

Component1.js

import { getFormattedExperiment } from 'hooks/useDatasetAction'

const Component1 = ({ accession,  downloadableSamples }) => {
  return (
    <AnotherComponent data={formatExperiment(accession, downloadableSamples)} />
  )
}

Component2.js

import { useDatasetAction } from 'hooks/useDatasetAction'

const Component2 = (data, otherData) => {
   const { getAddedSamples } = useDatasetAction(data, otherData)
   return ( ... )
}

As you can see the hook gets "instantiated" in the same way as before.

Please move them into the same file for now. I expect that we will want to rework this in the future so we aren't doing the formatting outside of the hook.

Ultimately, I believe that "formatting" the API response in a component outside of where we work with the data can hinder maintainability because we now need to identify which components are being provided "formatted" data. I am not asking for that rework today, just that we tuck this logic in that file together with the "processing" to facilitate readability for the time being.

[nozomione]

Hmm, I'm not sure I agree about the antipattern here. Specifically not sure what "since hooks are meant to be initialized when they're used" refers to here. We would continue to use the hook as we do only we would keep methods specific to preparing data for these hooks inside the same file.

useDatasetActions.js

export const getFormattedExperiments = () => {}
export const getFormattedExperimentsList = () => {}
export const useDatasetAction = () => {}

Component1.js

import { getFormattedExperiment } from 'hooks/useDatasetAction'

const Component1 = ({ accession,  downloadableSamples }) => {
  return (
    <AnotherComponent data={formatExperiment(accession, downloadableSamples)} />
  )
}

Component2.js

import { useDatasetAction } from 'hooks/useDatasetAction'

const Component2 = (data, otherData) => {
   const { getAddedSamples } = useDatasetAction(data, otherData)
   return ( ... )
}

As you can see the hook gets "instantiated" in the same way as before.

I was referring to how we instantiate the hook (doc: https://react.dev). Technically we are able to use any hooks as you suggested (after all, JS module import). But React hook should always be instantiated as I previously described (first importing the hook and then calling it to destruct its property).

I can move those methods back to useDatasetAction hook, but please keep the hook rule (we also use it throughout the codebase and in all react projects).

[davidsmejia]

Move the new helpers into the same file and name that file "formatDatasetAction". Then this looks good to merge. Again, would like to be able to identify the purpose of the formatting and this help in that effort.

[nozomione]

I've applied the feedback at 00b45439624eb67680fc6e19e60c18d1eec3d6a8 ✅ and ready for merge. Let me know, thank you @davidsmejia .