Skip to content

Conversation

@adamlazik1
Copy link
Contributor

What are the changes introduced in this pull request?

When creating new organization in the UI, the user has the option to input a custom organization label.

Considerations taken when implementing this change?

The label field was removed probably accidentally in #10880.
This action is still doable in API so it should be doable in UI as well.

What are the testing steps for this pull request?

  1. In the web UI, navigate to Administer > Organizations.
  2. Click New Organization.
  3. Input Name and custom Label and submit.
  4. Verify that the inputted label is present by clicking on the organization and displaying its details.

The label field was removed probably accidentally in Katello#10880.
This action is still doable in api so it should be doable in UI as well.
@adamruzicka
Copy link
Member

Looks good, the field is there, label gets derived from the name if the field is left blank, if the label is given explicitly it gets saved.

On the other hand, if you give a value and that value is invalid (contains spaces for example), it shows bad URI(is not URI?): "https://localhost:23443/candlepin/owners/something else" error instead of a proper validation error. I'd be willing to treat that as a separate issue

@chris1984 chris1984 self-assigned this Dec 3, 2024
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

ACK, code looks fine and works correctly:
label

I agree with Adam about the validation error, do you mind filing a redmine issue @adamlazik1

@adamruzicka
Copy link
Member

do you mind filing a redmine issue @adamlazik1

Adam is out, here's the redmine https://projects.theforeman.org/issues/38063

@chris1984 chris1984 merged commit d3a9089 into Katello:master Dec 5, 2024
@adamlazik1 adamlazik1 deleted the reintroduce-label branch January 6, 2025 11:39
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.

3 participants