Skip to content

[redcap] Add redcap{@}true to imported REDCap instruments meta#10926

Open
MaximeBICMTL wants to merge 1 commit into
aces:29.0-releasefrom
MaximeBICMTL:redcap-instruments-meta-redcap-true
Open

[redcap] Add redcap{@}true to imported REDCap instruments meta#10926
MaximeBICMTL wants to merge 1 commit into
aces:29.0-releasefrom
MaximeBICMTL:redcap-instruments-meta-redcap-true

Conversation

@MaximeBICMTL

@MaximeBICMTL MaximeBICMTL commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Implementation of https://github.com/aces/CBIGR/issues/831

Differentiate REDCap instruments from non-REDCap instruments by adding redcap{@}true to the formers meta file.

@MaximeBICMTL
MaximeBICMTL requested a review from regisoc July 16, 2026 10:13
@github-actions github-actions Bot added the Language: PHP PR or issue that update PHP code label Jul 16, 2026
@MaximeBICMTL MaximeBICMTL added Difficulty: Trivial PR or issue whose implementation is obviously correct and no-risk (typo, formatting...) Module: redcap PR or issue related to redcap module and removed Language: PHP PR or issue that update PHP code labels Jul 16, 2026
@MaximeBICMTL MaximeBICMTL changed the title Add redcap{@}true to imported REDCap instruments meta [redcap] Add redcap{@}true to imported REDCap instruments meta Jul 16, 2026

@regisoc regisoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Pretty simple, but is there no logic associated with this?
E.g. Instrument method to filter REDCap instruments.

@MaximeBICMTL

Copy link
Copy Markdown
Contributor Author

Nope I don't know the REDCap-specific logic that @ridz1208 needs, but this can be added in a later PR. In any case it makes sense to have a marker to differentiate REDCap and non-REDCap instruments.

@driusan

driusan commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

I don't think it makes sense to have this without the code changes to go with it. If it's a CBIGR override, it should have code changes that go along with it? There's no point in having a flag that's unused.. it also won't be brought along with instruments that are uploaded from the instrument uploader since it's in the meta file and not the linst file.

@MaximeBICMTL

Copy link
Copy Markdown
Contributor Author

Hhhm, this PR is not blocking for me so no biggie if not merged. But if I look at it as a standalone, I still think it is valuable addition to the REDCap module.

The REDCap-specific code seems to be implemented in #9472 (https://github.com/aces/CBIGR/pull/463 on C-BIG).

However, the point of this PR is to provide an answer to "was this instrument imported from REDCap", which AFAIK is currently not possible to answer and is IMO a valuable distinction. To do so, I think adding the redcap@true flag to the meta file is low-risk and makes sense:

  • If the meta file or the flag is absent then it can be assumed that the instrument was not imported from REDCap.
  • It respects the file format syntax and should be safely ignored by existing code.
  • It is already used in C-BIG.

Also, merging this PR somewhat early (before eventual code changes) allows more newly imported instruments to have this flag, preventing us from needing to go back and edit already imported instruments to add the flag manually as we've done on C-BIG.

Anyway, as I said this PR is not blocking for me, so feel free to move it to LORIS 30 or close it you prefer.

@ridz1208

Copy link
Copy Markdown
Collaborator

@driusan @MaximeBICMTL I'm neither here nor there on this one, I do see value in having the flag on it's own but there are some extremely low hanging fruit code changes that could be made alongside it.

  1. the data dictionary builder needs a case statement to ignore that filed like this (https://github.com/aces/Loris/blob/main/tools/exporters/data_dictionary_builder.php#L169)
  2. The reason we added that flag in the first place, the LINST class needs a rule addition to NOT ignore _status fields coming from redcap instruments (https://github.com/aces/Loris/blob/main/php/libraries/NDB_BVL_Instrument_LINST.class.inc#L403)
                if (substr($fieldname, -7) == "_status" && !$this->redcap) {

@MaximeBICMTL

MaximeBICMTL commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Ohokay, I was not aware of which exact changes used this flag, I can definitely add those to this PR though.

Just need to decide whether to include this on 29 or not, but note that I don't have access to a REDCap instance right now (token expired, need project to update, hence also why I am not reviewing other REDCap PRs 😬).

@ridz1208

Copy link
Copy Markdown
Collaborator

@MaximeBICMTL please add these little changes and make it to 29

merci

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Difficulty: Trivial PR or issue whose implementation is obviously correct and no-risk (typo, formatting...) Module: redcap PR or issue related to redcap module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants