Skip to content

Allow using dash and underscore in domains #3961

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

Draft
wants to merge 11 commits into
base: development
Choose a base branch
from

Conversation

samaradel
Copy link
Contributor

@samaradel samaradel commented Mar 5, 2025

Description

  • Allow using dash and underscore in domains
  • Update models decorators
  • Replace characters in workload names
  • Update Domains page and manage gateway dialog with new validators

Changes

Screencast.from.05-03-25.12.14.34.webm

image

Related Issues

Tested Scenarios

  • Add a dash or underscore in the domain
  • Adding a dash and an underscore as the first/last character is not acceptable
  • Add 2 or more underscores in the same domain name
  • Add 2 or more dashes in the same domain name
  • Combine 2 underscores and 2 dashes in the same domain name

Documentation PR

For UI changes, Please provide the Documentation PR on info_grid

To consider

Preliminary Checks:

  • Preliminary Checks
    • Does it completely address the issue linked?
    • What about edge cases?
    • Does it meet the specified acceptance criteria?
    • Are there any unintended side effects?
    • Does the PR adhere to the team's coding conventions, style guides, and best practices?
    • Does it integrate well with existing features?
    • Does it impact the overall performance of the application?
    • Are there any bottlenecks or slowdowns?
    • Has it been optimized for efficiency?
    • Has it been adequately tested with unit, integration, and end-to-end tests?
    • Are there any known defects or issues?
    • Is the code well-documented?
    • Are changes to documentation reflected in the code?

UI Checks:

  • UI Checks
    • If a UI design is provided/ does it follow it?
    • Does every button work?
    • Is the data displayed logical? Is it what you expected?
    • Does every validation work?
    • Does this interface feel intuitive?
    • What about slow network? Offline?
    • What if the gridproxy/graphql/chain is failing?
    • What would a first time user have a hard time navigating here?

Code Quality Checks:

  • Code Quality Checks
    • Code formatted/linted? Are there unused imports? .. etc
    • Is there redundant/repeated code?
    • Are there conditionals that are always true or always false?
    • Can we write this more concisely?
    • Can we reuse this code? If yes, where?
    • Will the changes be easy to maintain and update in the future?
    • Can this code become too complex to understand for other devs?
    • Can this code cause future integration problems?

Testing Checklist

  • Does it Meet the specified acceptance criteria.
  • Test if changes can affect any other functionality.
  • Tested with unit, integration, and end-to-end tests.
  • Tested the un-happy path and rollback scenarios.
  • Documentation updated to meet the latest changes.
  • Check automated tests working successfully with the changes.
  • Can be covered by automated tests.

General Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstring
  • Screenshots/Video attached (needed for UI changes)

Update models decorators
Replace characters in workload names
Update Domains page and manage gateway dialog with new validators
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2025-03-05.at.2.23.33.PM.mov

I can't delete the domain with dash and underscore while the old domains got deleted normally

also in the details I got the workload name is that expected ?

image

@samaradel
Copy link
Contributor Author

samaradel commented Mar 5, 2025

For deleting domains, I can't check atm as I added a domain from the client and it sound like it's not supported yet to show on the dashboard, so created an issue for this #3962

For the domain, it's just like what you added except the workload name. The dash is replaced as it's not supported in zos.

@samaradel samaradel requested a review from 0oM4R March 6, 2025 02:46
@samaradel samaradel marked this pull request as draft March 6, 2025 10:39
- Display FQDN instead of workload name
@samaradel samaradel marked this pull request as ready for review March 9, 2025 13:14
Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

can use underscore and dashes in subdomain name now
image
however the dash doesn't show in the name, only in the domain field
image
after deleting gateway, contract remains
image
I only see this in the logs
Start deleting the gateway deployment with name vm10732_vmjixu2n
even though domain is deleted from list here
image

@samaradel samaradel marked this pull request as draft March 12, 2025 08:37
- Display domain instead of workload name in the dialog
- Remove name from thr table
@samaradel samaradel marked this pull request as ready for review March 12, 2025 10:12
@0oM4R
Copy link
Contributor

0oM4R commented Mar 12, 2025

Listing and deleting work fine.
but name is still exist and it is very confusing
image
and what if we have another domain with the same name but without the dash
image

@samaradel
Copy link
Contributor Author

I think I should remove it form details (first Image) but I'm not sure about the rest
@AhmedHanafy725 what do you think?

@AhmedHanafy725
Copy link
Contributor

Listing and deleting work fine. but name is still exist and it is very confusing image

you can remove the name from these details

and what if we have another domain with the same name but without the dash image

I didn't get what is the issue (the domain is different)

@0oM4R
Copy link
Contributor

0oM4R commented Mar 19, 2025

and what if we have another domain with the same name but without the dash image

I didn't get what is the issue (the domain is different)

Confusing if the workload name is visible anywhere

@AhmedHanafy725 AhmedHanafy725 requested review from ramezsaeed and removed request for maayarosama March 19, 2025 09:27
Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

was able to create domains with dashes and underscore, validation works and checks if name exists , deletion also works, great job
Screenshot 2025-03-23 at 05 40 28

@0oM4R
Copy link
Contributor

0oM4R commented Apr 2, 2025

I still can see the workload name after removing the dash
image

Copy link
Contributor

@ramezsaeed ramezsaeed left a comment

Choose a reason for hiding this comment

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

Test scenarios:

  • add 2 or more underscores in the same domain name.
  • add 2 or more dashes in the same domain name
  • combine 2 underscores and 2 dashes in the same domain name
  • test delete domains with dashes and underscore

@samaradel samaradel marked this pull request as draft April 6, 2025 11:01
@samaradel samaradel marked this pull request as ready for review April 6, 2025 12:02
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

image

still in domains solution

@samaradel samaradel marked this pull request as draft April 6, 2025 15:22
@samaradel samaradel requested a review from 0oM4R April 6, 2025 15:28
@samaradel samaradel marked this pull request as ready for review April 6, 2025 15:28
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2025-04-07.at.11.49.56.AM.mov

I deployed a domain with a custom domain, but I couldn't delete it.
I think this case needs to be handled

@samaradel samaradel marked this pull request as draft April 7, 2025 10:39
@ramezsaeed
Copy link
Contributor

Screen.Recording.2025-04-07.at.11.49.56.AM.mov
I deployed a domain with a custom domain, but I couldn't delete it. I think this case needs to be handled

I can see the domain name doesn't have dashes or underscore, why it can't be deleted!

- Fix gateway before deploy using custom domain
@samaradel samaradel marked this pull request as ready for review April 8, 2025 14:30
@samaradel
Copy link
Contributor Author

@0oM4R @ramezsaeed fixed now

@samaradel samaradel marked this pull request as draft April 8, 2025 14:32
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