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

Feature/BI-750 Create an interface to list all studies that exist for a program #94

Merged
merged 30 commits into from
Jun 28, 2021

Conversation

dmeidlin
Copy link
Contributor

@dmeidlin dmeidlin commented Jun 3, 2021

Models, DAO, and Services created for Trials and Studies.
Route, views and component tables created for Trials and Studies.

Copy link
Member

@timparsons timparsons left a comment

Choose a reason for hiding this comment

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

It looks like code is there to display experiments, but a route only exists for trials. Was this intentional?

Comment on lines 18 to 20
// import {ProgramObservationLevel} from "@/breeding-insight/model/ProgramObservationLevel";
// import {Method} from "@/breeding-insight/model/Method";
// import {Scale} from "@/breeding-insight/model/Scale";
Copy link
Member

Choose a reason for hiding this comment

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

Are these needed for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, removed.

Comment on lines 18 to 20
// import {ProgramObservationLevel} from "@/breeding-insight/model/ProgramObservationLevel";
// import {Method} from "@/breeding-insight/model/Method";
// import {Scale} from "@/breeding-insight/model/Scale";
Copy link
Member

Choose a reason for hiding this comment

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

Are these needed for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

}

try {
if(!programId) throw new Error('mnissing or invalid program id');
Copy link
Member

Choose a reason for hiding this comment

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

"missing" is misspelled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! fixed.

data: T
-->
<template v-slot:side-panel="{tableRow}">
<TraitDetailPanel
Copy link
Member

Choose a reason for hiding this comment

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

Is this suppose to be the StudyDetailPanel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. changed

- limitations under the License.
-->

<template>
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be a clone of the TraitDetailPanel, I assume this will be properly implemented with BI-751?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct.

v-on:show-error-notification="$emit('show-error-notification', $event)"
>
<template v-slot="validations">
<BaseTraitForm
Copy link
Member

Choose a reason for hiding this comment

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

Should this be BaseStudyForm?

<template>
<div class="columns is-multiline">
<div class="column" v-bind:class="{'is-full': editFormat}">
<BasicInputField
Copy link
Member

Choose a reason for hiding this comment

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

I assume this will be updated to be specific to studies in BI-751?

ProgramUsersTable
}
})
export default class ProgramUserManagement extends ProgramsBase {
Copy link
Member

Choose a reason for hiding this comment

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

Class name doesn't match file name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per our conversation, experiments has been replaced with studies. All studies are now listed under "Trials and Studies", and the studies for a single trial can be listed by clicking on the name of a trial in the trial list.

TrialsTable
}
})
export default class ProgramLocationManagement extends TrialsBase {
Copy link
Member

Choose a reason for hiding this comment

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

Class name doesn't match file name

])
}
})
export default class ProgramManagement extends ProgramsBase {
Copy link
Member

Choose a reason for hiding this comment

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

Class name doesn't match file name

Copy link
Member

@timparsons timparsons left a comment

Choose a reason for hiding this comment

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

Looks good! Left a comment to remove commented out code, but approving otherwise.

Comment on lines +27 to +29
//import TrialsAndExperiments from "@/views/trials-and-studies/TrialsAndExperiments.vue";
import TrialsAndStudies from "@/views/trials-and-studies/TrialsAndStudies.vue";
//import Experiments from '@/views/trials-and-studies/Experiments.vue';
Copy link
Member

Choose a reason for hiding this comment

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

Can the commented out lines be removed?

@nickpalladino nickpalladino merged commit 54c1496 into develop Jun 28, 2021
@nickpalladino nickpalladino deleted the feature/BI-750 branch June 28, 2021 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants