Repository navigation
Conversation
Points the drag-and-drop container accessor at the list's class instead of its data-test-selector attribute, which is not meant to be rendered in production. https://community.openproject.org/wp/DREAM-906
Makes PermissionButtonComponent derive its wrapper key from the test_selector system argument instead of the data attribute, and passes the selector that way from all four callers. https://community.openproject.org/wp/DREAM-906
Applies the TestSelectorSystemArgument autocorrect: component constructors, slots and flex_layout receive test_selector: instead of a data-test-selector data attribute, so the attribute is no longer rendered in production. Primer form controls keep the data attribute, because they render the system argument literally. https://community.openproject.org/wp/DREAM-906
Generated by 🚫 Danger |
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
Runs erb_lint with autocorrect over the templates touched by this branch and commits the result. Layout and quote style only, apart from two ternaries with a nil branch in the PDF export styles partial, which become if expressions.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
https://community.openproject.org/wp/DREAM-906
What are you trying to accomplish?
Primer's
test_selector:system argument rendersdata-test-selectoronly outside production. Many callers pass the selector throughdata: { test_selector: }ordata: { "test-selector": }instead, which bypasses that and ships the attribute in production HTML.Converts the 199 such usages that sit on component constructors, slots and
flex_layoutto the system argument. Outside production the rendered HTML is unchanged, so no spec selector changes.What approach did you choose and why?
The conversion is the autocorrect output of a new cop,
OpenProject/TestSelectorSystemArgument, proposed in opf/rubocop-openproject#12. RuboCop applies it to Ruby files and erb_lint's RuboCop bridge to templates. Enabling the cop here follows in a further commit once that gem version is released; until then this PR stays a draft.Two places read the attribute at runtime and are changed first, in their own commits:
[data-test-selector=…]. It now uses the list's existing class.Shares::PermissionButtonComponentderived its Turbo Stream wrapper key fromdata[:"test-selector"]. It now reads thetest_selectorsystem argument, which is present in every environment; only its rendering is limited to non-production.Deliberately left as they are:
Primer::Alpha::TextField,TextArea,CheckBox,RadioButton,Select, form DSL inputs such asselect_list, and slots that build inputs). They do not honourtest_selector:and render it as a literaltest_selector="…"attribute. This was checked by rendering each affected component and slot shape both ways and comparing which element carries the attribute; all other shapes put it on the same element either way.component_wrapper,link_to,content_tag,primer_form_with, …), where no system argument exists.menu_optionsinSortHelper, which merges a top-leveldata:intocontent_arguments, so a top-leveltest_selector:would land on a different element.Not covered by CI: 39 of the converted selectors are not referenced by any spec. Their call shapes are among those rendered and compared as described above.
AI involvement
Merge checklist