feat(web): RMT registration #2541
Conversation
joseivanlopez
force-pushed
the
registration-url
branch
6 times, most recently
from
1c455c4 to
9940bdc
Compare
joseivanlopez
force-pushed
the
registration-url
branch
2 times, most recently
from
a78edca to
72cb81e
Compare
dgdavid
force-pushed
the
registration-url
branch
from
3d1fc83 to
19d0add
Compare
| <Form id={formId} onSubmit={submit}> | ||
| {/* // TRANSLATORS: input field label */} | ||
| <FormGroup label={_("Registration code")}> | ||
| <FormGroup fieldId={inputId} label={_("Registration code")}> |
There was a problem hiding this comment.
I've updated the id value here to ensure proper association between the label and its corresponding input. This was identified while writing unit tests using getByLabelText, which relies on that association. We're using this method specifically for password fields, since they can't be queried by role because they has no implicit role. Read testing-library/dom-testing-library#567
| </FormGroup> | ||
| <ActionGroup> | ||
| <Button | ||
| id={`register-button-${extension.id}-${extension.version}`} |
There was a problem hiding this comment.
For consistency with other ids, I moved this one into a variable too. That said, I’m not sure if it’s still necessary since it seems like it was originally added for testing purposes, but I have updated the test to avoid relying on ids. I’ve kept it for now to be safe, but it might be worth revisiting and potentially removing later.
| describe("and they are registered", () => { | ||
| beforeEach(() => { | ||
| registeredAddonInfoMock = [ | ||
| { | ||
| id: "sle-ha", | ||
| version: "16.0", | ||
| registrationCode: "INTERNAL-USE-ONLY-1234-ad42", | ||
| }, | ||
| ]; | ||
| }); | ||
|
|
||
| it("renders them with its registration code partially hidden", async () => { | ||
| const { user } = installerRender(<ProductRegistrationPage />, { withL10n: true }); | ||
|
|
||
| // the second "Show" button, the first one belongs to the base product registration code | ||
| const visibilityCodeToggler = screen.getAllByRole("button", { name: "Show" })[1]; | ||
| expect(visibilityCodeToggler).not.toBeNull(); | ||
|
|
||
| // only the end of the code is displayed | ||
| screen.getByText(/\*+ad42/); | ||
| // not the full code | ||
| expect(screen.queryByText(registeredAddonInfoMock[0].registrationCode)).toBeNull(); | ||
|
|
||
| // after pressing the "Show" button | ||
| await user.click(visibilityCodeToggler); | ||
| // the full code is visible | ||
| screen.getByText(registeredAddonInfoMock[0].registrationCode); | ||
| }); |
There was a problem hiding this comment.
NP: While updating this, I had the feeling that such a test actually belongs to the missing RegistrationExtensions.test.tsx
| const [server, setServer] = useState<ServerOption>("default"); | ||
| const [url, setUrl] = useState(""); | ||
| const [key, setKey] = useState(""); | ||
| const [email, setEmail] = useState(""); | ||
| const [provideKey, setProvideKey] = useState(true); | ||
| const [provideEmail, setProvideEmail] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [requestError, setRequestError] = useState<string | null>(null); | ||
| const [errors, setErrors] = useState<string[]>([]); |
There was a problem hiding this comment.
NP: Not a blocking comment but just a suggestion for the future: when a component or form starts managing a large number of state variables, I feel can be cleaner and more maintainable to use a reducer instead.
- If RMT registration was tried first, then SCC url has to be provided. Otherwise, the RMT url is still used.
joseivanlopez
force-pushed
the
registration-url
branch
from
19d0add to
147bc71
Compare
| </FormGroup> | ||
|
|
||
| {isProvided && ( | ||
| <NestedContent margin="mxMd" aria-live="polite"> |
There was a problem hiding this comment.
here I have to say that I do not get what that margin value means and AI was surprisingly helpful with explaning what it means. So it is In essence, margin="mxMd" tells the component to apply a predefined "medium" horizontal margin to itself.
There was a problem hiding this comment.
You just had to look at PatternFly documentation 😉 But since it is a kind of common pattern, AI was able to tell you the right answer 😄
There was a problem hiding this comment.
See https://www.patternfly.org/utility-classes/spacing#documentation
That said, it's true we can also improve a little bit the Nested content documentation for at least linking the above link
Prepare to release Agama 17: * #2459 * #2492 * #2526 * #2527 * #2528 * #2529 * #2530 * #2531 * #2532 * #2533 * #2534 * #2536 * #2537 * #2538 * #2540 * #2541 * #2542 * #2543 * #2544 * #2545 * #2546 * #2547 * #2548 * #2550 * #2551 * #2553 * #2554 * #2555 * #2556 * #2557 * #2558 * #2559 * #2560 * #2561 * #2562 * #2563 * #2564 * #2565 * #2566 * #2567 * #2568 * #2570 * #2571 * #2572 * #2575 * #2576 * #2577 * #2579 * #2580 * #2583 * #2584 * #2585 * #2586 * #2587 * #2588 * #2589 * #2591
5 participants
Add support for RMT in the web UI.