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.

Sign up for GitHub

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

Jump to bottom

feat(legal entity): extended legal entity length #860

Merged
merged 17 commits into from
Aug 14, 2024
Merged

feat(legal entity): extended legal entity length #860

merged 17 commits into from
Aug 14, 2024

Conversation

typecastcloud
Copy link
Contributor

@typecastcloud typecastcloud commented Jul 23, 2024 •
edited
Loading

Description

BE adjustment for: portal-frontend-registration:216

Extended pattern with additional currency characters and length adjustment.

Min length: 1
Max lenth: 160

Allows " (double quote \x22) to follow back-end implementation.
Allows £$€¥¢ to allow some additional currency symbols that are used in existing companies.

Why

Much longer company names exist than currently allowed.

Example:
https://find-and-update.company-information.service.gov.uk/company/03249311
88 characters

Issue

Refs: eclipse-tractusx/portal#360

Checklist

  • I have followed the contributing guidelines
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have added tests that prove my changes work
  • I have checked that new and existing tests pass locally with my changes
  • I have commented my code, particularly in hard-to-understand areas

evegufy
evegufy previously requested changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

@typecastcloud thank you for the contribution!
2.1.0 is closed for feature changes, could you please rebase to main?

@typecastcloud typecastcloud changed the base branch from release/v2.1.0-RC2 to main July 24, 2024 09:42
@dhiren-singh-007 dhiren-singh-007 force-pushed the feat/legal-entity-name-length branch from e63ec91 to 513b6c3 Compare July 24, 2024 09:51
@typecastcloud typecastcloud marked this pull request as ready for review July 25, 2024 08:25
Phil91
Phil91 requested changes Jul 26, 2024
View reviewed changes
Copy link
Member

@Phil91 Phil91 left a comment

Choose a reason for hiding this comment

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

Besides my nuget package comment no further changes are required in my opinion.

src/administration/Administration.Service/BusinessLogic/CompanyDataBusinessLogic.cs Outdated Show resolved Hide resolved
src/administration/Administration.Service/BusinessLogic/InvitationBusinessLogic.cs Outdated Show resolved Hide resolved
src/framework/Framework.Models/Directory.Build.props Outdated Show resolved Hide resolved
@dhiren-singh-007
Copy link
Contributor

dhiren-singh-007 commented Jul 26, 2024 •
edited
Loading

Besides my nuget package comment no further changes are required in my opinion.

Thanks @Phil91 for your review and suggestions.
I also think that it is good to change exception message with the new approach so I have changed the code accordingly.
Also i updated the packages by using script , please review it again.

@typecastcloud typecastcloud requested a review from Phil91 July 26, 2024 14:43
@dhiren-singh-007 dhiren-singh-007 force-pushed the feat/legal-entity-name-length branch from 5ab519a to a6922fa Compare August 1, 2024 14:04
@evegufy
Copy link
Contributor

evegufy commented Aug 5, 2024

Hi @dhiren-singh-007 @Phil91 pre-checks for nuget packages are failing https://github.com/eclipse-tractusx/portal-backend/actions/runs/10199854820/job/28336547514?pr=860

@dhiren-singh-007 dhiren-singh-007 force-pushed the feat/legal-entity-name-length branch from a6922fa to fab9962 Compare August 5, 2024 11:30
@dhiren-singh-007
Copy link
Contributor

Hi @dhiren-singh-007 @Phil91 pre-checks for nuget packages are failing https://github.com/eclipse-tractusx/portal-backend/actions/runs/10199854820/job/28336547514?pr=860

Thanks @evegufy , i rebased the main and updated the packages. Now its running fine

@evegufy evegufy requested a review from MaximilianHauer August 6, 2024 09:08
@dhiren-singh-007 dhiren-singh-007 requested a review from Phil91 August 6, 2024 12:50
Phil91
Phil91 previously approved these changes Aug 6, 2024
View reviewed changes
typecastcloud and others added 11 commits August 8, 2024 11:23
@dhiren-singh-007 dhiren-singh-007 force-pushed the feat/legal-entity-name-length branch from f92a89b to a906833 Compare August 8, 2024 09:33
@dhiren-singh-007 dhiren-singh-007 requested a review from Phil91 August 8, 2024 09:33
@MaximilianHauer MaximilianHauer added the priority PR needs to prioritized at review label Aug 12, 2024
@ybidois
Copy link

ybidois commented Aug 12, 2024

@MaximilianHauer thanks for setting this with higher priority! We really need it. Is it possible to approve it before Thursday?

Phil91
Phil91 previously approved these changes Aug 13, 2024
View reviewed changes
Copy link
Member

@Phil91 Phil91 left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks for changing the error handling to the new translatable method 👍

@MaximilianHauer MaximilianHauer added this to the Release 2.2.0 milestone Aug 13, 2024
@MaximilianHauer
Copy link

@evegufy will review today . @nicoprow please provide feedback if this is fine from BPDM perspective.

ntruchsess
ntruchsess requested changes Aug 13, 2024
View reviewed changes
src/framework/Framework.Models/ValidationExpressionsValidator.cs Outdated Show resolved Hide resolved
src/administration/Administration.Service/BusinessLogic/CompanyDataBusinessLogic.cs Outdated Show resolved Hide resolved
src/administration/Administration.Service/BusinessLogic/NetworkBusinessLogic.cs Outdated Show resolved Hide resolved
src/administration/Administration.Service/BusinessLogic/RegistrationBusinessLogic.cs Outdated Show resolved Hide resolved
src/administration/Administration.Service/BusinessLogic/RegistrationBusinessLogic.cs Outdated Show resolved Hide resolved
src/marketplace/Apps.Service/BusinessLogic/AppReleaseBusinessLogic.cs Outdated Show resolved Hide resolved
src/marketplace/Apps.Service/BusinessLogic/AppReleaseBusinessLogic.cs Outdated Show resolved Hide resolved
src/marketplace/Apps.Service/BusinessLogic/AppsBusinessLogic.cs Outdated Show resolved Hide resolved
src/marketplace/Services.Service/BusinessLogic/ServiceBusinessLogic.cs Outdated Show resolved Hide resolved
tests/framework/Framework.Models.Tests/ValidationExpressionsTests.cs Outdated Show resolved Hide resolved
@dhiren-singh-007
Copy link
Contributor

Thanks @ntruchsess for your review and suggestions . I applied the suggestions . Can you please re-review

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

nicoprow
nicoprow reviewed Aug 14, 2024
View reviewed changes
Copy link

@nicoprow nicoprow left a comment

Choose a reason for hiding this comment

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

The changes are compatible to the BPDM API as far as I can tell. Refer to here for more details.

@MaximilianHauer
Copy link

@ntruchsess / @evegufy you can proceed as soon as your findings are solved.

@ntruchsess ntruchsess dismissed evegufy’s stale review August 14, 2024 07:29

PR has been rebased on main, commit-history looks fine

@ntruchsess
Copy link
Contributor

ntruchsess commented Aug 14, 2024 •
edited
Loading

@dhiren-singh-007 @evegufy technically the PR is fine, but there is an issue with ECA-check, can you please check https://api.eclipse.org/git/eca/status/gh/eclipse-tractusx/portal-backend/860

@evegufy
Copy link
Contributor

evegufy commented Aug 14, 2024

@dhiren-singh-007 @evegufy technically the PR is fine, but there is an issue with ECA-check, can you please check https://api.eclipse.org/git/eca/status/gh/eclipse-tractusx/portal-backend/860

see https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/4897

@evegufy evegufy modified the milestones: Release 2.2.0, Release 24.12 Aug 14, 2024
@dhiren-singh-007
Copy link
Contributor

Thanks @evegufy and @ntruchsess . Now ECA check is passed

@evegufy evegufy merged commit 4c39a2f into eclipse-tractusx:main Aug 14, 2024
11 checks passed
@JordanGerada3 JordanGerada3 mentioned this pull request Aug 15, 2024
Merged
3 tasks
Phil91 added a commit to AnuragNagpure/portal-backend that referenced this pull request Sep 6, 2024
@typecastcloud @dhiren-singh-007 @Phil91
feat(legal entity): extended legal entity length (eclipse-tractusx#860)
dff4d0d 
* feat(company names): Increase company name length to align with front-end

Length increase is required for many companies. There are also companies with special characters at the start. Back-end was already misaligned with frontend-registration.

Refs: eclipse-tractusx/portal#360

---------

Co-authored-by: Dhirender Singh <[email protected]>
Co-authored-by: Dhirender Singh (Cofinity-X) <[email protected]>
Co-authored-by: Phil Schneider <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhiren-singh-007 dhiren-singh-007 dhiren-singh-007 left review comments

@nicoprow nicoprow nicoprow left review comments

@evegufy evegufy evegufy approved these changes

@ntruchsess ntruchsess ntruchsess approved these changes

@Phil91 Phil91 Phil91 left review comments

@MaximilianHauer MaximilianHauer Awaiting requested review from MaximilianHauer

Assignees
No one assigned
Labels
priority PR needs to prioritized at review
Projects
Portal
Status: USER READY
Milestone
Release 2.3.0 (24.12)
Development

Successfully merging this pull request may close these issues.
8 participants
@typecastcloud @dhiren-singh-007 @evegufy @ybidois @MaximilianHauer @ntruchsess @Phil91 @nicoprow