-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
[Improvement][ui] improving to find current version identifier(#15815) #15933
base: dev
Are you sure you want to change the base?
Conversation
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/Version.java
Outdated
Show resolved
Hide resolved
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/VersionMapper.java
Outdated
Show resolved
Hide resolved
@@ -30,4 +32,6 @@ public interface UiPluginService { | |||
|
|||
Map<String, Object> queryUiPluginDetailById(int id); | |||
|
|||
Map<String, Object> queryProductInfo(User loginUser, int userId); |
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.
Please use a specific object rather than `Map<String, Object> as the service method result.
Is this PR want to solve #15875 ? |
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.
I can't find the code for the UI part. Where is it?
The front-end code has also been completed and will be submitted after the back-end is error free |
Features like this need to be committed at both the backend and frontend. |
Okay, then I will modify the interface return class and resubmit the front-end and back-end code |
+1 |
...heduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/UiPluginController.java
Outdated
Show resolved
Hide resolved
...ler-api/src/test/java/org/apache/dolphinscheduler/api/controller/UiPluginControllerTest.java
Outdated
Show resolved
Hide resolved
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.
some nip
// * @param loginUser login user | ||
// * @param userId token for user |
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.
// * @param loginUser login user | |
// * @param userId token for user |
@Autowired | ||
DsVersionMapper dsVersionMapper; | ||
|
||
@Autowired | ||
private DsVersionDao dsVersionDao; |
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.
It's better to use dao in service but not mapper.
@Autowired | |
DsVersionMapper dsVersionMapper; | |
@Autowired | |
private DsVersionDao dsVersionDao; | |
@Autowired | |
private DsVersionDao dsVersionDao; |
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.
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.
Personally, I feel that there is no need to use a separate page to display the current version. You can put the version information at the top of this drop-down menu. |
Okay, I have removed the unused front-end references. According to my original plan, after completing the version function, document help, problem report submission, policy instructions, and service terms will also be placed on this page. If the version information is placed separately in the homepage column and other functions are added, it will appear crowded |
...heduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/UiPluginController.java
Outdated
Show resolved
Hide resolved
...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UiPluginServiceImpl.java
Outdated
Show resolved
Hide resolved
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/ProductInfoDto.java
Outdated
Show resolved
Hide resolved
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UiPluginService.java
Outdated
Show resolved
Hide resolved
...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UiPluginServiceImpl.java
Outdated
Show resolved
Hide resolved
...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UiPluginServiceImpl.java
Outdated
Show resolved
Hide resolved
...ler-api/src/test/java/org/apache/dolphinscheduler/api/controller/UiPluginControllerTest.java
Outdated
Show resolved
Hide resolved
...heduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/UiPluginController.java
Fixed
Show fixed
Hide fixed
dsVersion = dsVersionDao.selectVersion().map(DsVersion::getVersion).orElse("unknown"); | ||
if(StringUtils.isBlank(dsVersion)){ | ||
throw new ServiceException(Status.VERSION_INFO_STATE_ERROR); | ||
} |
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.
This is strange, if the t_ds_version is empty will use unknown, but if the version empty will throw exception.
* @return product info | ||
*/ | ||
@Operation(summary = "queryProductInfo", description = "QUERY_PRODUCT_INFO") | ||
@GetMapping(value = "/queryProductInfo") |
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.
@GetMapping(value = "/queryProductInfo") | |
@GetMapping(value = "/query-product-info") |
|
||
/** | ||
* obtain project version and address | ||
* @return product info | ||
*/ |
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.
/** | |
* obtain project version and address | |
* @return product info | |
*/ | |
/** | |
* obtain project version and address | |
* @return product info | |
*/ |
/** | ||
* ProductInfoDto | ||
*/ |
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.
/** | |
* ProductInfoDto | |
*/ | |
/** | |
* ProductInfoDto | |
*/ |
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.
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.
This comment is meaningless, you need to remove.
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
|
||
import static org.apache.dolphinscheduler.api.enums.Status.*; |
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.
Avoid wildcard import.
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.
Please use mvn spotless:apply to fix these.
Purpose of the pull request
close #15815
Brief change log
The version identifier of dolphinscheduler cannot be found on the UI. Please add the current version identifier on the UI.
在WEB 的UI界面上 无法找到当前运行的 dolphinscheduler 版本信息,无法确定当前运行的版本。
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
at testQueryProductInfo() method of UiPluginControllerTest class
(or)
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md