Skip to content

Conversation

@labkey-bpatel
Copy link
Contributor

@labkey-bpatel labkey-bpatel commented Mar 5, 2025

Rationale

Issue 49544: Deprecate the ability to select specific objects to import and apply import to multiple folders under the advanced import options

Keep 'Advanced Import Options' behind deprecated flag until 25.7, then we will remove it. I had already opened a PR where I had removed this option, can use it for reference.

Related Pull Requests

Changes

  • Move 'Advanced Import Options' from admin-importFolder.view behind 'Deprecated' flag.
  • Move 'Show advanced import options' from pipeline-startFolderImport.view behind 'Deprecated' flag.
  • Display 'Create shared datasets' and 'Fail import for undefined visits' with tooltips on admin-importFolder.view page.

Copy link
Contributor

@labkey-klum labkey-klum left a comment

Choose a reason for hiding this comment

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

These changes look good but there is also the check box for advanced options that is available from the folder settings import tab that should be put behind the flag:

image

@labkey-bpatel
Copy link
Contributor Author

labkey-bpatel commented Mar 6, 2025

Hi @labkey-klum - The reason I did not put that checkbox behind the flag since checking it displays below page, where there are two more options here 'Validate Queries' and 'Study Import Options' - should those be put behind the flag as well? Also, Validate Queries seems redundant here, since its already present in the previous step.

image

@labkey-bpatel labkey-bpatel requested a review from cnathe March 20, 2025 18:35
…er Import (via Process and Import Data > select folder zip > Import Data > Import Folder)
…Renderable as an argument, use DOM builder to generate server side HTML to ensure proper encoding. Also, keep 'Fail import of undefined visits' as-is when importing from a Pipeline route.
Copy link
Contributor

@labkey-adam labkey-adam left a comment

Choose a reason for hiding this comment

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

See comments about not adding a JspBase method and not double encoding the help text

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants