-
Notifications
You must be signed in to change notification settings - Fork 32
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
fix(service marketplace): list all active services #1134
fix(service marketplace): list all active services #1134
Conversation
db2dae4
to
f8a893a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine to me. lgtm
@ma3u @oyo can you please review this PR ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Usmanfee thank you for your contribution. Could you please update the DEPENDENCIES file? https://github.com/eclipse-tractusx/portal-frontend/actions/runs/10996801570?pr=1134
@oyo could you please review?
@evegufy I have updated the dependency file but the error message still persists and no any clue with the error message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use hard coded string values
49a99db
to
952b257
Compare
@manojava-gk I have introduced enum for frequently used string for sorting type . can you have a look please ? :) |
@Usmanfee Just to maintain consistency use PascalCase instead. See other examples in the page. |
@manojava-gk we are also using camelcase format in e.g. |
@Usmanfee one more thing, constants file is a place where we host most common things in the app. I do not think this sorting type is a generic one. it is very specific to the back end api. I prefer to define this in the specific api types's file. In future we can define this in the constants when all apis sorting types are unified. CC: @oyo |
@manojava-gk I have updated the code based on your suggestion. could you please have a look now ? Thank you :) |
Looks fine now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few changes especially it's not a good pattern to handle undefined cases with an empty string. That was in the code before and went in unnoticed but now it would be a good occasion to clean that up.
@oyo Thanks for your feedback :) . I have resolved your comments with suggested changes. |
76f6a73
to
00cc69f
Compare
Quality Gate passedIssues Measures |
Description
Service marketplace should have all the active services but now with implemented solution we can see all the services.
Changelog entry:
- fixed service marketplace to display all active available servicesp[#1143](https://github.com/eclipse-tractusx/portal-frontend/issues/1143)
Why
Service Marketplace hasn't have the all the active services available throughout from all the service provider. Now, for each service provider marketplace we can observe all the services.
Issue
#1143
Checklist