[Survey module] Make public module #3609
Conversation
…roll to top on next and prev
…tatic fields diplay values if present
| @@ -0,0 +1,60 @@ | |||
| <?php | |||
| /** | |||
| * Implements the main LORIS user login page to handle authentication | |||
There was a problem hiding this comment.
This comment and the one below should be updated
| static function formatElement($element, $elName) | ||
| { | ||
| $tempElement = array( | ||
| "Type" => $element['type'], |
There was a problem hiding this comment.
How do you make sure the key 'type' exists in the array?
Is the function intended to "eat" LorisFormElement ?
There was a problem hiding this comment.
Yeah, this class is designed so that it could hypothetically replace the current way we convert the instrument to JSON for the API. The inputted element is expected to be in the format outputted by LorisForm which as far as I know always has the type set.
There was a problem hiding this comment.
Could you type hint $element to \LorisFormElement?
|
|
||
| } | ||
|
|
||
| ?> |
| */ | ||
| function getReactReview() | ||
| { | ||
| $DB = Database::singleton(); |
There was a problem hiding this comment.
Please use NDB_Factory->database()
| import Page from './DirectEntryForm'; | ||
|
|
||
| /*eslint-disable */ | ||
| class DirectEntry extends React.Component { |
There was a problem hiding this comment.
The indentation should be 2 spaces
There was a problem hiding this comment.
2 spaces is only used to reduce size of javascript files. Since this is compiled into a minified file I feel like 4 spaces is better suited as it aligns with our standards for php along with making this file easier to read
There was a problem hiding this comment.
But 4 spaces do not align with our JSX standard and make files harder to read
| // Since the Instrument data is set when the component is | ||
| // mounted we want to display nothing until it has been set | ||
| return ( | ||
| <div></div> |
There was a problem hiding this comment.
A loading span would be nice.
There was a problem hiding this comment.
I am making a Loader Component I hope to make a pull request for soon.
| </div> | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
please add proptypes so that reviewData is described properly.
There was a problem hiding this comment.
Default props would also be useful.
| render() { | ||
|
|
||
| let questions = this.props.reviewData.questions.map((element) => { | ||
| console.log(element); |
| let questions = this.props.reviewData.questions.map((element) => { | ||
| console.log(element); | ||
| return ( | ||
| <tr className='reviewPage'> |
|
|
||
| componentWillMount() { | ||
|
|
||
| $.ajax({ |
There was a problem hiding this comment.
This should be done in ComponentDidMount because ComponentWillMoiunt is done just before the first render and this.setState might not trigger a second rendering whit the new state. Otherwise, the method name is now UNSAFE_componentWillMount()
| * @license Loris license | ||
| * @link https://www.github.com/aces/Loris/ | ||
| */ | ||
| class DirectDataEntryMainPage |
There was a problem hiding this comment.
htdocs/survey.php still exists
|
|
||
| unset($Values['CommentID']); | ||
| unset($Values['UserID']); | ||
| // unset($Values['Testdate']); |
There was a problem hiding this comment.
Is this meant to be un-commented eventually, or should it be removed?
|
|
||
| $this->tpl_data['Values'] = json_encode($Values); | ||
|
|
||
| echo json_encode($this->tpl_data); |
There was a problem hiding this comment.
Since the entirety of $this->tpl_data is being json_encoded, is the json_encode on 129 necessary? I believe json_encode works recursively.
| dataType: 'json', | ||
| success: function(data) { | ||
| const InstrumentJSON = JSON.parse(data.InstrumentJSON); | ||
| const Values = JSON.parse(data.Values); |
There was a problem hiding this comment.
Should these be parsed? Are they not already encoded before being returned by the ajax call and therefore already in the proper format to be set as JSON objects?
| // Since the Instrument data is set when the component is | ||
| // mounted we want to display nothing until it has been set | ||
| return ( | ||
| <div></div> |
There was a problem hiding this comment.
I am making a Loader Component I hope to make a pull request for soon.
| class ReviewPage extends React.Component { | ||
| constructor(props) { | ||
| super(props); | ||
| } |
There was a problem hiding this comment.
If no functions need to be bound or state's set, then the constructor can be removed.
| </div> | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Default props would also be useful.
| /** | ||
| * This is used by the Loris survey module to retrieve the email | ||
| * template for the current instrument. It is used in the survey_accounts | ||
| * page via AJAX to update the email template with the current page |
|
|
||
| /** | ||
| * Handles a request by delegating to the appropriate | ||
| * handle method |
There was a problem hiding this comment.
should have an ending dot to stay consistant with other comments
| $values = array(); | ||
|
|
||
| foreach ($temp->questions as $value) { | ||
| // $this->form->directValues[$value->SourceField] = $value->response; |
| /** | ||
| * This file contains React component for Direct Data Entry | ||
| * | ||
| * @author Jordan Stirling (StiringApps ltd.) |
There was a problem hiding this comment.
I'm not sure about advertising in the codebase... but in any case you've got a typo here 😸
|
@jstirling91 in general we are trying to use return type declarations for new PHP code. Would you be able to add these to your function signatures? Should be quick. given that they're already documented with return types. |
driusan
added
the
Needs Maintainer
|
@driusan @ridz1208 |
|
@zaliqarosli lets talk about this one |
|
This needs to be discussed with Dave, Rida, Tom Beaudry. |
|
Had a meeting with @driusan @thomasbeaudry & @driusan regarding how to handle this PR; |
driusan
added
Needs Work
|
See related Issue for status of this |
This pull request creates a public module structure for the survey module. It redesigns the old version to use React using API like calls to retrieve, partially save and validate forms.