[timepoint_list] Candidate Info button appears with proper permissions #9696
Conversation
| $this->tpl_data['isDataEntryPerson'] = $user->hasCenterPermission("data_entry", $cand_CenterID); | ||
|
|
There was a problem hiding this comment.
@adamdaudrich why change that line if you havent changed its functionality at all ?
There was a problem hiding this comment.
not changed, I was playing with these class variables to learn their behaviour and re-typed it.
There was a problem hiding this comment.
In PRs going to the core we try to avoid unecessary changes like this because it distracts from the actual changes and may cause static errors (which this is most likely doing because of the line length rule). if its not causing an error its fine for now, if it is, undo it so the error is resolved.
| {/if} | ||
| {if $isImagingPerson} | ||
| <a class="btn btn-default" role="button" href="{$baseurl}/imaging_browser/?DCCID={$candID}">View Imaging datasets</a> | ||
| {/if} | ||
| {if $isCandidatePerson} |
There was a problem hiding this comment.
Although this makes logical sense, its not backwards compatible.
Personally I would opt for this but the team might have a different view
@driusan @racostas @CamilleBeau @SantiagoTG any opinion on this?
There was a problem hiding this comment.
I'm pro this change. It makes sense to me that the button would only show if users have a permission to view candidate info
There was a problem hiding this comment.
I would call the variable hasCandidateParameterAccess because I find "isCandidatePerson" a little confusing but I see why its named that for symmetry with the others..
There was a problem hiding this comment.
Other than that, I don't see why anyone would be opposed to this change. What backwards compatibility is lost? A button that goes to a permission denied page?
adamdaudrich
changed the title
Brief summary of changes
a new smarty variable "isCandidatePerson" was added in order to isolate the display of the
Candidate InfoButton. The .tpl file was adjusted accordinglyHave you updated related documentation? Not yet
Testing instructions
User permissions settings:
Candidate InfoButtonCandidate Infobutton appears.Link(s) to related issue(s)[](
#9585 (comment))