Commit d68436d9 authored by Clement Ho's avatar Clement Ho Committed by Filipa Lacerda

Delete epic

parent cef76488
...@@ -29,6 +29,11 @@ export default { ...@@ -29,6 +29,11 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
showDeleteButton: {
type: Boolean,
required: false,
default: true,
},
issuableRef: { issuableRef: {
type: String, type: String,
required: true, required: true,
...@@ -92,6 +97,11 @@ export default { ...@@ -92,6 +97,11 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
issuableType: {
type: String,
required: false,
default: 'issue',
},
}, },
data() { data() {
const store = new Store({ const store = new Store({
...@@ -157,21 +167,21 @@ export default { ...@@ -157,21 +167,21 @@ export default {
}) })
.catch(() => { .catch(() => {
eventHub.$emit('close.form'); eventHub.$emit('close.form');
window.Flash('Error updating issue'); window.Flash(`Error updating ${this.issuableType}`);
}); });
}, },
deleteIssuable() { deleteIssuable() {
this.service.deleteIssuable() this.service.deleteIssuable()
.then(res => res.json()) .then(res => res.json())
.then((data) => { .then((data) => {
// Stop the poll so we don't get 404's with the issue not existing // Stop the poll so we don't get 404's with the issuable not existing
this.poll.stop(); this.poll.stop();
gl.utils.visitUrl(data.web_url); gl.utils.visitUrl(data.web_url);
}) })
.catch(() => { .catch(() => {
eventHub.$emit('close.form'); eventHub.$emit('close.form');
window.Flash('Error deleting issue'); window.Flash(`Error deleting ${this.issuableType}`);
}); });
}, },
}, },
...@@ -223,6 +233,7 @@ export default { ...@@ -223,6 +233,7 @@ export default {
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
:project-path="projectPath" :project-path="projectPath"
:project-namespace="projectNamespace" :project-namespace="projectNamespace"
:show-delete-button="showDeleteButton"
/> />
<div v-else> <div v-else>
<title-component <title-component
......
...@@ -13,6 +13,11 @@ ...@@ -13,6 +13,11 @@
type: Object, type: Object,
required: true, required: true,
}, },
showDeleteButton: {
type: Boolean,
required: false,
default: true,
},
}, },
data() { data() {
return { return {
...@@ -23,6 +28,9 @@ ...@@ -23,6 +28,9 @@
isSubmitEnabled() { isSubmitEnabled() {
return this.formState.title.trim() !== ''; return this.formState.title.trim() !== '';
}, },
shouldShowDeleteButton() {
return this.canDestroy && this.showDeleteButton;
},
}, },
methods: { methods: {
closeForm() { closeForm() {
...@@ -62,7 +70,7 @@ ...@@ -62,7 +70,7 @@
Cancel Cancel
</button> </button>
<button <button
v-if="canDestroy" v-if="shouldShowDeleteButton"
class="btn btn-danger pull-right append-right-default" class="btn btn-danger pull-right append-right-default"
:class="{ disabled: deleteLoading }" :class="{ disabled: deleteLoading }"
type="button" type="button"
......
...@@ -36,6 +36,11 @@ ...@@ -36,6 +36,11 @@
type: String, type: String,
required: true, required: true,
}, },
showDeleteButton: {
type: Boolean,
required: false,
default: true,
},
}, },
components: { components: {
lockedWarning, lockedWarning,
...@@ -81,6 +86,7 @@ ...@@ -81,6 +86,7 @@
:markdown-docs-path="markdownDocsPath" /> :markdown-docs-path="markdownDocsPath" />
<edit-actions <edit-actions
:form-state="formState" :form-state="formState"
:can-destroy="canDestroy" /> :can-destroy="canDestroy"
:show-delete-button="showDeleteButton" />
</form> </form>
</template> </template>
...@@ -35,6 +35,11 @@ export default { ...@@ -35,6 +35,11 @@ export default {
type: String, type: String,
required: false, required: false,
}, },
containerClass: {
type: String,
required: false,
default: 'btn btn-align-content',
},
}, },
components: { components: {
loadingIcon, loadingIcon,
...@@ -49,9 +54,9 @@ export default { ...@@ -49,9 +54,9 @@ export default {
<template> <template>
<button <button
class="btn btn-align-content"
@click="onClick" @click="onClick"
type="button" type="button"
:class="containerClass"
:disabled="loading || disabled" :disabled="loading || disabled"
> >
<transition name="fade"> <transition name="fade">
......
...@@ -353,3 +353,7 @@ ...@@ -353,3 +353,7 @@
display: -webkit-flex; display: -webkit-flex;
display: flex; display: flex;
} }
.flex-right {
margin-left: auto;
}
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
# when the user is destroyed. # when the user is destroyed.
module Users module Users
class MigrateToGhostUserService class MigrateToGhostUserService
prepend EE::Users::MigrateToGhostUserService
extend ActiveSupport::Concern extend ActiveSupport::Concern
attr_reader :ghost_user, :user attr_reader :ghost_user, :user
......
---
title: Add delete epic button
merge_request:
author:
type: added
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
import userAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import userAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
import timeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import timeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import tooltip from '~/vue_shared/directives/tooltip'; import tooltip from '~/vue_shared/directives/tooltip';
import loadingButton from '~/vue_shared/components/loading_button.vue';
import { s__ } from '~/locale';
export default { export default {
name: 'epicHeader', name: 'epicHeader',
...@@ -15,6 +17,16 @@ ...@@ -15,6 +17,16 @@
type: String, type: String,
required: true, required: true,
}, },
canDelete: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
deleteLoading: false,
};
}, },
directives: { directives: {
tooltip, tooltip,
...@@ -22,6 +34,15 @@ ...@@ -22,6 +34,15 @@
components: { components: {
userAvatarLink, userAvatarLink,
timeagoTooltip, timeagoTooltip,
loadingButton,
},
methods: {
deleteEpic() {
if (confirm(s__('Epic will be removed! Are you sure?'))) { // eslint-disable-line no-alert
this.deleteLoading = true;
this.$emit('deleteEpic');
}
},
}, },
}; };
</script> </script>
...@@ -29,21 +50,26 @@ ...@@ -29,21 +50,26 @@
<template> <template>
<div class="detail-page-header"> <div class="detail-page-header">
<div class="issuable-meta"> <div class="issuable-meta">
Opened {{ s__('Opened') }}
<timeagoTooltip <timeago-tooltip :time="created" />
:time="created" {{ s__('by') }}
/>
by
<strong> <strong>
<user-avatar-link <user-avatar-link
:link-href="author.url" :link-href="author.url"
:img-src="author.src" :img-src="author.src"
:img-size="24" :img-size="24"
:tooltipText="author.username" :tooltip-text="author.username"
:username="author.name" :username="author.name"
imgCssClasses="avatar-inline" img-css-classes="avatar-inline"
/> />
</strong> </strong>
</div> </div>
<loading-button
v-if="canDelete"
:loading="deleteLoading"
@click="deleteEpic"
:label="s__('Delete')"
container-class="btn btn-remove btn-inverted flex-right"
/>
</div> </div>
</template> </template>
<script> <script>
import issuableApp from '~/issue_show/components/app.vue'; import issuableApp from '~/issue_show/components/app.vue';
import issuableAppEventHub from '~/issue_show/event_hub';
import epicHeader from './epic_header.vue'; import epicHeader from './epic_header.vue';
import epicSidebar from '../../sidebar/components/sidebar_app.vue'; import epicSidebar from '../../sidebar/components/sidebar_app.vue';
...@@ -65,16 +66,23 @@ ...@@ -65,16 +66,23 @@
required: false, required: false,
}, },
}, },
data() {
return {
// Epics specific configuration
issuableRef: '',
projectPath: this.groupPath,
projectNamespace: '',
};
},
components: { components: {
epicHeader, epicHeader,
epicSidebar, epicSidebar,
issuableApp, issuableApp,
}, },
created() { methods: {
// Epics specific configuration deleteEpic() {
this.issuableRef = ''; issuableAppEventHub.$emit('delete.issuable');
this.projectPath = this.groupPath; },
this.projectNamespace = '';
}, },
}; };
</script> </script>
...@@ -84,6 +92,8 @@ ...@@ -84,6 +92,8 @@
<epic-header <epic-header
:author="author" :author="author"
:created="created" :created="created"
:canDelete="canDestroy"
@deleteEpic="deleteEpic"
/> />
<div class="issuable-details content-block"> <div class="issuable-details content-block">
<div class="detail-page-description"> <div class="detail-page-description">
...@@ -105,9 +115,21 @@ ...@@ -105,9 +115,21 @@
</div> </div>
<epic-sidebar <epic-sidebar
:endpoint="endpoint" :endpoint="endpoint"
:issuable-ref="issuableRef"
:initial-title-html="initialTitleHtml"
:initial-title-text="initialTitleText"
:initial-description-html="initialDescriptionHtml"
:initial-description-text="initialDescriptionText"
:markdown-preview-path="markdownPreviewPath"
:markdown-docs-path="markdownDocsPath"
:project-path="projectPath"
:project-namespace="projectNamespace"
:show-inline-edit-button="true"
:show-delete-button="false"
issuable-type="epic"
:editable="canUpdate" :editable="canUpdate"
:initialStartDate="startDate" :initial-start-date="startDate"
:initialEndDate="endDate" :initial-end-date="endDate"
/> />
</div> </div>
</div> </div>
......
...@@ -6,11 +6,7 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -6,11 +6,7 @@ document.addEventListener('DOMContentLoaded', () => {
const metaData = JSON.parse(el.dataset.meta); const metaData = JSON.parse(el.dataset.meta);
const initialData = JSON.parse(el.dataset.initial); const initialData = JSON.parse(el.dataset.initial);
const props = Object.assign({}, initialData, metaData, { const props = Object.assign({}, initialData, metaData);
// Current iteration does not enable users
// to delete epics
canDestroy: false,
});
// Convert backend casing to match frontend style guide // Convert backend casing to match frontend style guide
props.startDate = props.start_date; props.startDate = props.start_date;
......
...@@ -27,13 +27,15 @@ module EE ...@@ -27,13 +27,15 @@ module EE
enable :create_epic enable :create_epic
enable :admin_epic enable :admin_epic
enable :update_epic enable :update_epic
enable :destroy_epic
end end
rule { owner }.enable :destroy_epic
rule { auditor }.policy do rule { auditor }.policy do
enable :read_group enable :read_group
enable :read_epic enable :read_epic
end end
rule { admin }.enable :read_epic rule { admin }.enable :read_epic
rule { has_projects }.enable :read_epic rule { has_projects }.enable :read_epic
......
module EE
module Users
module MigrateToGhostUserService
private
def migrate_records
migrate_epics
super
end
def migrate_epics
user.epics.update_all(author_id: ghost_user.id)
::Epic.where(last_edited_by_id: user.id).update_all(last_edited_by_id: ghost_user.id)
end
end
end
end
...@@ -91,7 +91,7 @@ describe Groups::EpicsController do ...@@ -91,7 +91,7 @@ describe Groups::EpicsController do
describe 'PUT #update' do describe 'PUT #update' do
before do before do
group.add_user(user, :developer) group.add_developer(user)
put :update, group_id: group, id: epic.to_param, epic: { title: 'New title' }, format: :json put :update, group_id: group, id: epic.to_param, epic: { title: 'New title' }, format: :json
end end
...@@ -107,7 +107,7 @@ describe Groups::EpicsController do ...@@ -107,7 +107,7 @@ describe Groups::EpicsController do
describe 'GET #realtime_changes' do describe 'GET #realtime_changes' do
subject { get :realtime_changes, group_id: group, id: epic.to_param } subject { get :realtime_changes, group_id: group, id: epic.to_param }
it 'returns epic' do it 'returns epic' do
group.add_user(user, :developer) group.add_developer(user)
subject subject
expect(response.content_type).to eq 'application/json' expect(response.content_type).to eq 'application/json'
...@@ -122,4 +122,25 @@ describe Groups::EpicsController do ...@@ -122,4 +122,25 @@ describe Groups::EpicsController do
end end
end end
end end
describe "DELETE #destroy" do
before do
sign_in(user)
end
it "rejects a developer to destroy an epic" do
group.add_developer(user)
delete :destroy, group_id: group, id: epic.to_param
expect(response).to have_gitlab_http_status(404)
end
it "deletes the epic" do
group.add_owner(user)
delete :destroy, group_id: group, id: epic.to_param
expect(response).to have_gitlab_http_status(302)
expect(controller).to set_flash[:notice].to(/The epic was successfully deleted\./)
end
end
end end
require 'spec_helper'
feature 'Delete Epic', :js do
let(:user) { create(:user) }
let(:group) { create(:group, :public) }
let(:epic) { create(:epic, group: group) }
let!(:epic2) { create(:epic, group: group) }
before do
sign_in(user)
end
context 'when user who is not a group member displays the epic' do
it 'does not show the Delete button' do
visit group_epic_path(group, epic)
expect(page).not_to have_selector('.detail-page-header button')
end
end
context 'when user with owner access displays the epic' do
before do
group.add_owner(user)
visit group_epic_path(group, epic)
wait_for_requests
end
it 'deletes the issue and redirect to epic list' do
page.accept_alert 'Epic will be removed! Are you sure?' do
find('.detail-page-header button').click
end
wait_for_requests
expect(find('.issuable-list')).not_to have_content(epic.title)
expect(find('.issuable-list')).to have_content(epic2.title)
end
end
end
...@@ -32,6 +32,14 @@ describe EpicPolicy do ...@@ -32,6 +32,14 @@ describe EpicPolicy do
it 'reporter group member can manage epics' do it 'reporter group member can manage epics' do
group.add_reporter(user) group.add_reporter(user)
expect(permissions(user, group)).to be_disallowed(:destroy_epic)
expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
end
it 'only group owner can destroy epics' do
group.add_owner(user)
expect(permissions(user, group)) expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic) .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
end end
...@@ -60,6 +68,14 @@ describe EpicPolicy do ...@@ -60,6 +68,14 @@ describe EpicPolicy do
it 'reporter group member can manage epics' do it 'reporter group member can manage epics' do
group.add_reporter(user) group.add_reporter(user)
expect(permissions(user, group)).to be_disallowed(:destroy_epic)
expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
end
it 'only group owner can destroy epics' do
group.add_owner(user)
expect(permissions(user, group)) expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic) .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
end end
...@@ -88,6 +104,14 @@ describe EpicPolicy do ...@@ -88,6 +104,14 @@ describe EpicPolicy do
it 'reporter group member can manage epics' do it 'reporter group member can manage epics' do
group.add_reporter(user) group.add_reporter(user)
expect(permissions(user, group)).to be_disallowed(:destroy_epic)
expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
end
it 'only group owner can destroy epics' do
group.add_owner(user)
expect(permissions(user, group)) expect(permissions(user, group))
.to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic) .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
end end
......
require 'spec_helper'
describe Users::MigrateToGhostUserService do
context 'epics' do
let!(:user) { create(:user) }
let(:service) { described_class.new(user) }
context 'deleted user is present as both author and edited_user' do
include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:author, :last_edited_by] do
let(:created_record) do
create(:epic, group: create(:group), author: user, last_edited_by: user)
end
end
end
context 'deleted user is present only as edited_user' do
include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:last_edited_by] do
let(:created_record) { create(:epic, group: create(:group), author: create(:user), last_edited_by: user) }
end
end
end
end
...@@ -31,4 +31,42 @@ describe('epicHeader', () => { ...@@ -31,4 +31,42 @@ describe('epicHeader', () => {
it('should render username tooltip', () => { it('should render username tooltip', () => {
expect(vm.$el.querySelector('.user-avatar-link span').dataset.originalTitle).toEqual(author.username); expect(vm.$el.querySelector('.user-avatar-link span').dataset.originalTitle).toEqual(author.username);
}); });
describe('canDelete', () => {
it('should not show loading button by default', () => {
expect(vm.$el.querySelector('.btn-remove')).toBeNull();
});
it('should show loading button if canDelete', (done) => {
vm.canDelete = true;
Vue.nextTick(() => {
expect(vm.$el.querySelector('.btn-remove')).toBeDefined();
done();
});
});
});
describe('delete epic', () => {
let deleteEpic;
beforeEach((done) => {
deleteEpic = jasmine.createSpy();
spyOn(window, 'confirm').and.returnValue(true);
vm.canDelete = true;
vm.$on('deleteEpic', deleteEpic);
Vue.nextTick(() => {
vm.$el.querySelector('.btn-remove').click();
done();
});
});
it('should set deleteLoading', () => {
expect(vm.deleteLoading).toEqual(true);
});
it('should emit deleteEpic event', () => {
expect(deleteEpic).toHaveBeenCalled();
});
});
}); });
...@@ -3,6 +3,8 @@ import epicShowApp from 'ee/epics/epic_show/components/epic_show_app.vue'; ...@@ -3,6 +3,8 @@ import epicShowApp from 'ee/epics/epic_show/components/epic_show_app.vue';
import epicHeader from 'ee/epics/epic_show/components/epic_header.vue'; import epicHeader from 'ee/epics/epic_show/components/epic_header.vue';
import epicSidebar from 'ee/epics/sidebar/components/sidebar_app.vue'; import epicSidebar from 'ee/epics/sidebar/components/sidebar_app.vue';
import issuableApp from '~/issue_show/components/app.vue'; import issuableApp from '~/issue_show/components/app.vue';
import issuableAppEventHub from '~/issue_show/event_hub';
import '~/lib/utils/url_utility';
import mountComponent from '../../../helpers/vue_mount_component_helper'; import mountComponent from '../../../helpers/vue_mount_component_helper';
import { props } from '../mock_data'; import { props } from '../mock_data';
import issueShowData from '../../../issue_show/mock_data'; import issueShowData from '../../../issue_show/mock_data';
...@@ -43,6 +45,7 @@ describe('EpicShowApp', () => { ...@@ -43,6 +45,7 @@ describe('EpicShowApp', () => {
headerVm = mountComponent(EpicHeader, { headerVm = mountComponent(EpicHeader, {
author, author,
created, created,
canDelete: canDestroy,
}); });
const IssuableApp = Vue.extend(issuableApp); const IssuableApp = Vue.extend(issuableApp);
...@@ -86,4 +89,14 @@ describe('EpicShowApp', () => { ...@@ -86,4 +89,14 @@ describe('EpicShowApp', () => {
it('should render epic-sidebar', () => { it('should render epic-sidebar', () => {
expect(vm.$el.innerHTML.indexOf(sidebarVm.$el.innerHTML) !== -1).toEqual(true); expect(vm.$el.innerHTML.indexOf(sidebarVm.$el.innerHTML) !== -1).toEqual(true);
}); });
it('should emit delete.issuable when epic is deleted', () => {
const deleteIssuable = jasmine.createSpy();
issuableAppEventHub.$on('delete.issuable', deleteIssuable);
spyOn(window, 'confirm').and.returnValue(true);
spyOn(gl.utils, 'visitUrl').and.callFake(() => {});
vm.$el.querySelector('.detail-page-header .btn-remove').click();
expect(deleteIssuable).toHaveBeenCalled();
});
}); });
export const contentProps = { export const contentProps = {
endpoint: '', endpoint: '',
canUpdate: true, canUpdate: true,
canDestroy: false, canDestroy: true,
markdownPreviewPath: '', markdownPreviewPath: '',
markdownDocsPath: '', markdownDocsPath: '',
groupPath: '', groupPath: '',
......
...@@ -223,23 +223,46 @@ describe('Issuable output', () => { ...@@ -223,23 +223,46 @@ describe('Issuable output', () => {
}); });
}); });
it('closes form on error', (done) => { describe('error when updating', () => {
spyOn(window, 'Flash').and.callThrough(); beforeEach(() => {
spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => { spyOn(window, 'Flash').and.callThrough();
reject(); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => {
})); reject();
}));
});
vm.updateIssuable(); it('closes form on error', (done) => {
vm.updateIssuable();
setTimeout(() => { setTimeout(() => {
expect( expect(
eventHub.$emit, eventHub.$emit,
).toHaveBeenCalledWith('close.form'); ).toHaveBeenCalledWith('close.form');
expect( expect(
window.Flash, window.Flash,
).toHaveBeenCalledWith('Error updating issue'); ).toHaveBeenCalledWith('Error updating issue');
done(); done();
});
});
it('returns the correct error message for issuableType', (done) => {
vm.issuableType = 'merge request';
Vue.nextTick(() => {
vm.updateIssuable();
setTimeout(() => {
expect(
eventHub.$emit,
).toHaveBeenCalledWith('close.form');
expect(
window.Flash,
).toHaveBeenCalledWith('Error updating merge request');
done();
});
});
}); });
}); });
}); });
......
...@@ -61,6 +61,15 @@ describe('Edit Actions components', () => { ...@@ -61,6 +61,15 @@ describe('Edit Actions components', () => {
}); });
}); });
it('should not show delete button if showDeleteButton is false', (done) => {
vm.showDeleteButton = false;
Vue.nextTick(() => {
expect(vm.$el.querySelector('.btn-danger')).toBeNull();
done();
});
});
describe('updateIssuable', () => { describe('updateIssuable', () => {
it('sends update.issauble event when clicking save button', () => { it('sends update.issauble event when clicking save button', () => {
vm.$el.querySelector('.btn-save').click(); vm.$el.querySelector('.btn-save').click();
......
...@@ -66,6 +66,23 @@ describe('LoadingButton', function () { ...@@ -66,6 +66,23 @@ describe('LoadingButton', function () {
}); });
}); });
describe('container class', () => {
it('should default to btn btn-align-content', () => {
vm = mountComponent(LoadingButton, {});
expect(vm.$el.classList.contains('btn')).toEqual(true);
expect(vm.$el.classList.contains('btn-align-content')).toEqual(true);
});
it('should be configurable through props', () => {
vm = mountComponent(LoadingButton, {
containerClass: 'test-class',
});
expect(vm.$el.classList.contains('btn')).toEqual(false);
expect(vm.$el.classList.contains('btn-align-content')).toEqual(false);
expect(vm.$el.classList.contains('test-class')).toEqual(true);
});
});
describe('click callback prop', () => { describe('click callback prop', () => {
it('calls given callback when normal', () => { it('calls given callback when normal', () => {
vm = mountComponent(LoadingButton, { vm = mountComponent(LoadingButton, {
......
...@@ -26,6 +26,7 @@ describe GroupPolicy do ...@@ -26,6 +26,7 @@ describe GroupPolicy do
:admin_namespace, :admin_namespace,
:admin_group_member, :admin_group_member,
:change_visibility_level, :change_visibility_level,
:destroy_epic,
(Gitlab::Database.postgresql? ? :create_subgroup : nil) (Gitlab::Database.postgresql? ? :create_subgroup : nil)
].compact ].compact
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment