-
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
|
||
@Data | ||
@TableName("t_ds_version") | ||
public class Version { |
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 already exist DsVersion
/** | ||
* user mapper interface | ||
*/ | ||
public interface VersionMapper extends BaseMapper<Version> { |
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.
Duplicate with DsVersionMapper
.
@@ -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
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UiPluginService.java
Fixed
Show fixed
Hide fixed
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.
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 |
* @return product info | ||
*/ | ||
@Operation(summary = "queryProductInfo", description = "QUERY_PRODUCT_INFO") | ||
@PostMapping(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.
Please use GetMapping
dsVersion = dsVersionDao.selectVersion().map(DsVersion::getVersion).orElse("unknown"); | ||
|
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.
These seems never changed, so it would be better to query only once when the api startup, this can reduce a lot of db query.
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.
@Data | ||
public class ProductInfoDto { | ||
|
||
private Integer id; |
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.
private Integer id; | |
private Integer id; |
Remove the useless code.
@@ -30,4 +32,6 @@ public interface UiPluginService { | |||
|
|||
Map<String, Object> queryUiPluginDetailById(int id); | |||
|
|||
ProductInfoDto queryProductInfo(User loginUser); |
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 queryProductInfo(User loginUser); | |
ProductInfoDto queryProductInfo(); |
@@ -82,4 +104,12 @@ public Map<String, Object> queryUiPluginDetailById(int id) { | |||
return result; | |||
} | |||
|
|||
@Override | |||
public ProductInfoDto queryProductInfo(User loginUser) { |
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.
public ProductInfoDto queryProductInfo(User loginUser) { | |
public ProductInfoDto queryProductInfo() { |
@@ -82,4 +104,12 @@ public Map<String, Object> queryUiPluginDetailById(int id) { | |||
return result; | |||
} | |||
|
|||
@Override | |||
public ProductInfoDto queryProductInfo(User loginUser) { | |||
// persist to the database |
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.
// persist to the database | |
// persist to the database |
private static final Logger logger = LoggerFactory.getLogger(TenantControllerTest.class); | ||
|
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.
private static final Logger logger = LoggerFactory.getLogger(TenantControllerTest.class); | |
private static final Logger logger = LoggerFactory.getLogger(TenantControllerTest.class); | |
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.
Okay, I have removed the code from the places you mentioned
@ResponseStatus(HttpStatus.OK) | ||
@ApiException(VERSION_INFO_STATE_ERROR) | ||
public Result<ProductInfoDto> queryProductInfo( | ||
@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
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