Skip to content
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.

Sign up for GitHub

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

Preset titles of duplicated templates and projects #4774

Merged
merged 1 commit into from
Oct 20, 2021
Merged

Preset titles of duplicated templates and projects #4774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Kitodo/src/main/java/org/kitodo/production/forms/ProjectForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ public String duplicate(Integer itemId) {
try {
this.baseProject = ServiceManager.getProjectService().getById(itemId);
this.project = ServiceManager.getProjectService().duplicateProject(baseProject);
this.setSaveDisabled(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is possible to do without "this" then leave it out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Is that a coding guideline of us? We use "this" all the time to reference methods and properties of the current instance of a class (like the previous lines)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also find it unnecessary. I have also observed that you use this a lot, and I often find it superfluous. The IDE marks class fields very clearly so that this is no longer needed since color monitors came in use. I actually only use this when setting class fields in constructors and setters, where the parameter has the same name as the field to set. At this point it is also not necessary to call the setter. Why not:

template = ServiceManager.getTemplateService().duplicateTemplate(baseTemplate);
assignedProjects.clear();
assignedProjects.addAll(template.getProjects());
saveDisabled = false;

Copy link
Collaborator

@markusweigelt markusweigelt Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atm it is not a guideline, but from my perspective it is redundant because you can also access current methods and variables without "this" keyword. Except for example ambiguity between instance variables and method parameters and such cases.

It is true that it is already used very often. I do not know this massive use from other projects but ok then I will approve the review if the test is successful and take the topic with me to clarify it.

return projectEditPath + "&referer=projects";
} catch (DAOException e) {
Helper.setErrorMessage(ERROR_DUPLICATE, new Object[] {ObjectType.PROJECT.getTranslationSingular() }, logger,
Expand Down
1 change: 1 addition & 0 deletions Kitodo/src/main/java/org/kitodo/production/forms/TemplateForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public String duplicate(Integer itemId) {
this.template = ServiceManager.getTemplateService().duplicateTemplate(baseTemplate);
this.assignedProjects.clear();
this.assignedProjects.addAll(template.getProjects());
this.setSaveDisabled(false);
return templateEditPath;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is possible to do without "this" then leave it out.

} catch (DAOException e) {
Helper.setErrorMessage(ERROR_DUPLICATE, new Object[] {ObjectType.TEMPLATE.getTranslationSingular() },
Expand Down
3 changes: 2 additions & 1 deletion Kitodo/src/main/java/org/kitodo/production/services/data/ProjectService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.kitodo.production.dto.ClientDTO;
import org.kitodo.production.dto.ProjectDTO;
import org.kitodo.production.dto.TemplateDTO;
import org.kitodo.production.helper.Helper;
import org.kitodo.production.services.ServiceManager;
import org.kitodo.production.services.data.base.ClientSearchService;
import org.primefaces.model.SortOrder;
Expand Down Expand Up @@ -258,7 +259,7 @@ public boolean isProjectComplete(Project project) {
public Project duplicateProject(Project baseProject) {
Project duplicatedProject = new Project();

// project title is intentionally not duplicated
duplicatedProject.setTitle(baseProject.getTitle() + "_" + Helper.generateRandomString(3));
duplicatedProject.setClient(baseProject.getClient());
duplicatedProject.setStartDate(baseProject.getStartDate());
duplicatedProject.setEndDate(baseProject.getEndDate());
Expand Down
2 changes: 1 addition & 1 deletion Kitodo/src/main/java/org/kitodo/production/services/data/TemplateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private List<TemplateDTO> findAvailableForAssignToUser(Integer projectId) throws
public Template duplicateTemplate(Template baseTemplate) {
Template duplicatedTemplate = new Template();

// Template _title_ should explicitly _not_ be duplicated!
duplicatedTemplate.setTitle(baseTemplate.getTitle() + "_" + Helper.generateRandomString(3));
duplicatedTemplate.setCreationDate(new Date());
duplicatedTemplate.setClient(baseTemplate.getClient());
duplicatedTemplate.setDocket(baseTemplate.getDocket());
Expand Down
6 changes: 3 additions & 3 deletions Kitodo/src/main/webapp/pages/projectEdit.xhtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
</script>
<h3 id="headerText">
<h:outputText value="#{msgs.editProject} (#{ProjectForm.project.title})"
rendered="#{not empty ProjectForm.project.title}"/>
rendered="#{ProjectForm.project.id ne null}"/>
<h:outputText value="#{msgs.newProject}"
rendered="#{empty ProjectForm.project.title and empty ProjectForm.project.folders}"/>
rendered="#{ProjectForm.project.id eq null and empty ProjectForm.project.title}"/>
<h:outputText value="#{msgs.duplicateProject}"
rendered="#{empty ProjectForm.project.title and not empty ProjectForm.project.folders}"/>
rendered="#{ProjectForm.project.id eq null and not empty ProjectForm.project.title}"/>
</h3>
<p:commandButton id="save"
widgetVar="save"
Expand Down
6 changes: 3 additions & 3 deletions Kitodo/src/main/webapp/pages/templateEdit.xhtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@

<!--@elvariable id="isCreateMode" type="boolean"-->
<ui:param name="isCreateMode"
value="#{TemplateForm.template.title eq '' and empty TemplateForm.template.docket and SecurityAccessController.hasAuthorityToAddTemplate()}"/>
value="#{TemplateForm.template.id eq null and empty TemplateForm.template.title and SecurityAccessController.hasAuthorityToAddTemplate()}"/>
<!--@elvariable id="isDuplicateMode" type="boolean"-->
<ui:param name="isDuplicateMode"
value="#{TemplateForm.template.title eq '' and not empty TemplateForm.template.docket and SecurityAccessController.hasAuthorityToAddTemplate()}"/>
value="#{TemplateForm.template.id eq null and not empty TemplateForm.template.title and SecurityAccessController.hasAuthorityToAddTemplate()}"/>
<!--@elvariable id="isEditMode" type="boolean"-->
<ui:param name="isEditMode"
value="#{not TemplateForm.currentTemplateInUse and TemplateForm.template.title ne '' and not empty TemplateForm.template.docket and SecurityAccessController.hasAuthorityToEditTemplate()}"/>
value="#{TemplateForm.template.id ne null and not TemplateForm.currentTemplateInUse and SecurityAccessController.hasAuthorityToEditTemplate()}"/>
<!--@elvariable id="isViewMode" type="boolean"-->
<ui:param name="isViewMode"
value="#{not isEditMode and not isDuplicateMode and not isCreateMode and SecurityAccessController.hasAuthorityToViewTemplate()}"/>
Expand Down