Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closes #803 and #804 - Users may now define custom promotion messages. #864

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CreateDialog extends React.Component {

return (
<Dialog
focusOnShow={false}
style={{ width: '400px' }}
header={'Create ' + type}
modal={true}
Expand Down Expand Up @@ -68,7 +69,10 @@ class CreateDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.input.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.input.current.element.focus();
}, 0);

const { filePath } = this.props;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class DeleteDialog extends React.Component {
return (
<Dialog
header={'Delete ' + type}
focusOnShow={false}
modal={true}
visible={this.props.visible}
onHide={this.props.onHide}
Expand All @@ -41,7 +42,10 @@ class DeleteDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.deleteButton.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.deleteButton.current.element.focus();
}, 0);

const { filePath } = this.props;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class DownloadDialog extends React.Component {
return (
<Dialog
header={'Download Configuration File'}
focusOnShow={false}
modal={true}
visible={this.props.visible}
onHide={this.props.onHide}
Expand Down Expand Up @@ -75,7 +76,10 @@ class DownloadDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.downloadButton.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.downloadButton.current.element.focus();
}, 0);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,12 @@ const PromotionView = () => {
/**
* Promotes the currently approved files.
*/
const promoteConfigurations = async () => {
const promoteConfigurations = async (commitMessage) => {
const payload = {
files: currentApprovals,
workspaceCommitId,
liveCommitId,
commitMessage,
};

setIsPromoting(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,92 @@ import React from 'react';
import { Dialog } from 'primereact/dialog';
import { Button } from 'primereact/button';
import { ProgressBar } from 'primereact/progressbar';
import { InputText } from 'primereact/inputtext';

/**
* Dialog for showing the currently approved files before promoting them.
*/
const PromotionApprovalDialog = ({ visible, onHide, onPromote, isLoading, approvedFiles = [] }) => {
const footer = (
<div>
<Button label="Promote" onClick={onPromote} disabled={isLoading} />
<Button label="Cancel" className="p-button-secondary" onClick={onHide} disabled={isLoading} />
</div>
);

return (
<>
<style jsx>
{`
.list li {
font-family: monospace;
}

.content :global(.p-progressbar) {
height: 0.5rem;
}
`}
</style>

<Dialog header="Promote Configurations" visible={visible} style={{ width: '50vw' }} modal={true} onHide={onHide} footer={footer}>
<div className="content">
<span>The following files have been approved and will be promoted:</span>
<ul className="list">
{approvedFiles.map((file) => (
<li key={file}>{file}</li>
))}
</ul>

{isLoading && <ProgressBar mode="indeterminate" />}
</div>
</Dialog>
</>
);
};
class PromotionApprovalDialog extends React.Component {
state = {
/** The promotion message to be send to the backend. */
promotionMessage: '',
};
input = React.createRef();
render() {
const { visible, onHide, isLoading, approvedFiles = [] } = this.props;
const { promotionMessage } = this.state;
const footer = (
<div>
<Button label="Promote" onClick={this.promote} disabled={isLoading || !promotionMessage} />
<Button label="Cancel" className="p-button-secondary" onClick={onHide} disabled={isLoading} />
</div>
);

return (
<>
<style jsx>
{`
.list li {
font-family: monospace;
}

.content :global(.p-progressbar) {
height: 0.5rem;
}
`}
</style>

<Dialog
header="Promote Configurations"
focusOnShow={false}
visible={visible}
style={{ width: '50vw' }}
modal={true}
onHide={onHide}
footer={footer}
>
<div className="content">
<span>The following files have been approved and will be promoted:</span>
<ul className="list">
{approvedFiles.map((file) => (
<li key={file}>{file}</li>
))}
</ul>

{isLoading && <ProgressBar mode="indeterminate" />}
</div>
<InputText
ref={this.input}
style={{ width: '100%' }}
placeholder="Describe change..."
onKeyPress={this.onKeyPress}
value={promotionMessage}
onChange={(e) => this.setState({ promotionMessage: e.target.value })}
/>
</Dialog>
</>
);
}

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.input.current.element.focus();
}, 0);
this.setState({ promotionMessage: '' });
}
}

onKeyPress = (e) => {
if (e.key === 'Enter' && !this.props.isLoading && this.state.promotionMessage) {
this.promote();
}
};

promote = () => {
this.props.onPromote(this.state.promotionMessage);
};
}

export default PromotionApprovalDialog;
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class CreateDialog extends React.Component {
return (
<Dialog
header={'Create User'}
focusOnShow={false}
modal={true}
visible={this.props.visible}
onHide={this.props.onHide}
Expand Down Expand Up @@ -99,7 +100,10 @@ class CreateDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.input.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.input.current.element.focus();
}, 0);
this.setState({ ...initialState });
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class DeleteDialog extends React.Component {
return (
<Dialog
header={'Delete User'}
focusOnShow={false}
modal={true}
visible={this.props.visible}
onHide={this.props.onHide}
Expand Down Expand Up @@ -50,7 +51,10 @@ class DeleteDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.deleteButton.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.deleteButton.current.element.focus();
}, 0);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class ClearDialog extends React.Component {
<Dialog
header={'Clear History of all Agents' + this.props.type}
modal={true}
focusOnShow={false}
visible={this.props.visible}
onHide={this.props.onHide}
footer={
Expand All @@ -37,7 +38,10 @@ class ClearDialog extends React.Component {

componentDidUpdate(prevProps) {
if (!prevProps.visible && this.props.visible) {
this.clearButton.current.element.focus();
/**Timeout is needed for .focus() to be triggered correctly. */
setTimeout(() => {
this.clearButton.current.element.focus();
}, 0);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ public void promoteConfiguration(ConfigurationPromotion promotion, boolean allow
}

// commit changes
commitFiles(getCurrentAuthor(), "Promoting configuration files", false);
commitFiles(getCurrentAuthor(), promotion.getCommitMessage(), false);

} catch (IOException | GitAPIException ex) {
throw new PromotionFailedException("Configuration promotion has failed.", ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
@Data
public class ConfigurationPromotion {

/**
* A short message describing the commit.
*/
@ApiModelProperty(example = "Added rules for connection tracing")
private String commitMessage;

/**
* Represents the id of the commit which holds the files to promote.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ public ResponseEntity<Object> handleSelfPromotion(Exception exception) {

@ApiOperation(value = "Fetch promotion files", notes = "Fetches all configuration files which are ready for promotion.")
@GetMapping(value = "configuration/promotions")
public WorkspaceDiff getPromotions(@ApiParam("Specifies whether the old and new content of each files should also be returned.") @RequestParam(defaultValue = "false", name = "include-content") boolean includeContent, Authentication authentication) throws IOException, GitAPIException {
public WorkspaceDiff getPromotions(
@ApiParam("Specifies whether the old and new content of each files should also be returned.")
@RequestParam(defaultValue = "false", name = "include-content")
boolean includeContent, Authentication authentication) throws IOException, GitAPIException {
WorkspaceDiff workspaceDiff = fileManager.getWorkspaceDiff(includeContent);
workspaceDiff.setCanPromoteOwnChanges(allowSelfPromotion(authentication));
return workspaceDiff;
Expand All @@ -53,8 +56,14 @@ public WorkspaceDiff getPromotions(@ApiParam("Specifies whether the old and new
@Secured(UserRoleConfiguration.PROMOTE_ACCESS_ROLE)
@ApiOperation(value = "Promote configurations", notes = "Promotes the specified configuration files.")
@PostMapping(value = "configuration/promote")
public ResponseEntity promoteConfiguration(@ApiParam("The definition that contains the information about which files to promote.") @RequestBody ConfigurationPromotion promotion, Authentication authentication) throws GitAPIException {
public ResponseEntity promoteConfiguration(
@ApiParam("The definition that contains the information about which files to promote.")
@RequestBody
ConfigurationPromotion promotion, Authentication authentication) throws GitAPIException {
boolean allowSelfPromotion = allowSelfPromotion(authentication);
if (promotion.getCommitMessage() == null || promotion.getCommitMessage().isEmpty()) {
return ResponseEntity.badRequest().build();
}
try {
fileManager.promoteConfiguration(promotion, allowSelfPromotion);
return ResponseEntity.ok().build();
Expand All @@ -67,7 +76,8 @@ private boolean allowSelfPromotion(Authentication authentication) {
if (!settings.getSecurity().isFourEyesPromotion()) {
return true;
}
return authentication.getAuthorities().stream()
return authentication.getAuthorities()
.stream()
.anyMatch(auth -> auth.getAuthority().equals(UserRoleConfiguration.ADMIN_ACCESS_ROLE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ public void getPromotionFiles() {
ResponseEntity<WorkspaceDiff> result = authRest.getForEntity("/api/v1/configuration/promotions", WorkspaceDiff.class);

assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(result.getBody().getEntries())
.containsExactly(SimpleDiffEntry.builder().file("/src/file.yml").type(DiffEntry.ChangeType.ADD)
.authors(Collections.singletonList(settings.getDefaultUser().getName())).build());
assertThat(result.getBody().getEntries()).containsExactly(SimpleDiffEntry.builder()
.file("/src/file.yml")
.type(DiffEntry.ChangeType.ADD)
.authors(Collections.singletonList(settings.getDefaultUser().getName()))
.build());
}
}

Expand All @@ -55,6 +57,7 @@ public void promoteFiles() {
promotion.setFiles(Collections.singletonList("/src/file.yml"));
promotion.setLiveCommitId(diff.getBody().getLiveCommitId());
promotion.setWorkspaceCommitId(diff.getBody().getWorkspaceCommitId());
promotion.setCommitMessage("test");

ResponseEntity<Void> result = authRest.exchange("/api/v1/configuration/promote", HttpMethod.POST, new HttpEntity<>(promotion), Void.class);

Expand All @@ -71,10 +74,45 @@ public void promoteNoFiles() {
promotion.setFiles(Collections.emptyList());
promotion.setLiveCommitId(diff.getBody().getLiveCommitId());
promotion.setWorkspaceCommitId(diff.getBody().getWorkspaceCommitId());
promotion.setCommitMessage("test");

ResponseEntity<Void> result = authRest.exchange("/api/v1/configuration/promote", HttpMethod.POST, new HttpEntity<>(promotion), Void.class);

assertThat(result.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
}

@Test
public void emptyPromotionMessage() {
authRest.exchange("/api/v1/files/src/file.yml", HttpMethod.PUT, null, Void.class);

ResponseEntity<WorkspaceDiff> diff = authRest.getForEntity("/api/v1/configuration/promotions", WorkspaceDiff.class);

ConfigurationPromotion promotion = new ConfigurationPromotion();
promotion.setFiles(Collections.emptyList());
promotion.setLiveCommitId(diff.getBody().getLiveCommitId());
promotion.setWorkspaceCommitId(diff.getBody().getWorkspaceCommitId());
promotion.setCommitMessage("");

ResponseEntity<Void> result = authRest.exchange("/api/v1/configuration/promote", HttpMethod.POST, new HttpEntity<>(promotion), Void.class);

assertThat(result.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}

@Test
public void nullPromotionMessage() {
authRest.exchange("/api/v1/files/src/file.yml", HttpMethod.PUT, null, Void.class);

ResponseEntity<WorkspaceDiff> diff = authRest.getForEntity("/api/v1/configuration/promotions", WorkspaceDiff.class);

ConfigurationPromotion promotion = new ConfigurationPromotion();
promotion.setFiles(Collections.emptyList());
promotion.setLiveCommitId(diff.getBody().getLiveCommitId());
promotion.setWorkspaceCommitId(diff.getBody().getWorkspaceCommitId());
promotion.setCommitMessage(null);

ResponseEntity<Void> result = authRest.exchange("/api/v1/configuration/promote", HttpMethod.POST, new HttpEntity<>(promotion), Void.class);

assertThat(result.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}
}
}