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

Change when plugin fieldset is hidden in registration variables table #5113

Conversation

viktorvanwijk
Copy link
Contributor

@viktorvanwijk viktorvanwijk commented Feb 19, 2025

Closes #4990

Changes

In the registration variables table, the plugin fieldset is now only hidden if there is only one configured backend, or if all the configured backends are of the same type.

Checklist

Check off the items that are completed or not relevant.

  • Impact on features

    • Checked copying a form
    • Checked import/export of a form
    • Config checks in the configuration overview admin page
    • Problem detection in the admin email digest is handled
  • Release management

    • I have labelled the PR as "needs-backport" accordingly
  • I have updated the translations assets (you do NOT need to provide translations)

    • Ran ./bin/makemessages_js.sh
    • Ran ./bin/compilemessages_js.sh
  • Dockerfile/scripts

    • Updated the Dockerfile with the necessary scripts from the ./bin folder
  • Commit hygiene

    • Commit messages refer to the relevant Github issue
    • Commit messages explain the "why" of change, not the how

…les table

Previously, the plugin fieldset was only shown when multiple configured backends introduced registration variables. This led to confusion when there were multiple backends configured, but only ONE of them introduced registration variables. In this case, the registration variables would not be grouped by their plugin name, and it was not clear to which backend they belonged.

Now, the fieldset will only be hidden if there is only ONE configured backend, OR if all the configured backends are of the same type (meaning they introduce the same registration variables).
@viktorvanwijk viktorvanwijk marked this pull request as ready for review February 19, 2025 14:28
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.73%. Comparing base (51c5bde) to head (1bb5d45).
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5113   +/-   ##
=======================================
  Coverage   96.73%   96.73%           
=======================================
  Files         774      774           
  Lines       26657    26657           
  Branches     3468     3468           
=======================================
  Hits        25787    25787           
  Misses        608      608           
  Partials      262      262           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@viktorvanwijk viktorvanwijk force-pushed the feature/4990-improve-registration-variables-table-in-admin branch from 8c1cff5 to 436ab3d Compare February 19, 2025 14:41
@sergei-maertens sergei-maertens self-requested a review February 20, 2025 11:05
@@ -369,11 +369,11 @@ export const WithObjectsAPIRegistrationBackends = {
await userEvent.click(registrationTab);

const pdfUrl = canvas.getByRole('cell', {name: 'pdf_url'});
expect(pdfUrl).toBeVisible();
await expect(pdfUrl).toBeVisible();
Copy link
Member

Choose a reason for hiding this comment

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

expect is always sync - possibly you need await canvas.findByRole('cell', {name: 'pdf_url'}); on the line above if there's flakiness due to async stuff happening

Copy link
Contributor Author

@viktorvanwijk viktorvanwijk Feb 20, 2025

Choose a reason for hiding this comment

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

I have gotten used to adding an await before expect calls because my IDE complains about it

@viktorvanwijk viktorvanwijk force-pushed the feature/4990-improve-registration-variables-table-in-admin branch from 436ab3d to 1bb5d45 Compare February 20, 2025 14:07
@sergei-maertens sergei-maertens merged commit f46fe6e into master Feb 21, 2025
33 checks passed
@sergei-maertens sergei-maertens deleted the feature/4990-improve-registration-variables-table-in-admin branch February 21, 2025 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve registration variables table in admin
2 participants