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.

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

Download the search result as a file in a selected format #2581

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

lbownik
Copy link

@lbownik lbownik commented Nov 6, 2024

No description provided.

@@ -101,6 +101,11 @@ public Long getId() {
public DatasetFieldType getDatasetFieldType() {
return datasetFieldType;
}

public boolean isOfType(final DatasetFieldType type) {

Copy link

Choose a reason for hiding this comment

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

This empty line do not fit Dataverse coding style. Should be removed.

@@ -110,6 +110,8 @@ public class DatasetFieldType implements Serializable, Comparable<DatasetFieldTy
private int displayOrder;

private String displayFormat;

private boolean exportToFile = false;
Copy link

Choose a reason for hiding this comment

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

Redundant assignment as false is be default.



public boolean isExportToFile() {

Copy link

Choose a reason for hiding this comment

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

Empty line do not fit overall code style. Please remove it.

@@ -506,6 +514,11 @@ public void setRequiredInDataverse(boolean requiredInDataverse) {
public void setDisplayFormat(String displayFormat) {
this.displayFormat = displayFormat;
}

public void setExportToFile(final boolean exportToFile) {

Copy link

Choose a reason for hiding this comment

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

Empty line, as above's. Please remove it.

@@ -205,6 +205,18 @@ public List<DatasetField> getDatasetFields() {
public List<DatasetField> getDatasetFieldsOptional() {
return datasetFieldsOptional;
}

public List<DatasetField> getDatasetFieldsAll() {

Copy link

Choose a reason for hiding this comment

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

Empty line. Please remove it.

solrSearchResultsService.populateDatafileSearchCard(
results.getOrDefault(SearchObjectType.FILES, Collections.emptyList()));
Copy link

Choose a reason for hiding this comment

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

Please revert such changes, as they do not bring valuable think.

if (sortOrderSupplied.equals(SortOrder.desc.toString())) {
this.sortOrder = SortOrder.desc;
}
public void setSortOrder(final String sortOrderSupplied) {
Copy link

Choose a reason for hiding this comment

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

Seems that it not change much. I would revert it, as it not add a value, and make tracking changes harder.

@@ -24,6 +24,8 @@
<ui:param name="showDataverseHeader" value="false"/>
<ui:define name="body">
<h:form id="dashboardMaximumEmbargoForm" onkeypress="return event.keyCode != 13">

<h1>eksport wyników wyszukiwania</h1>
Copy link

Choose a reason for hiding this comment

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

Why it is preceded with empty line? Please remove it.
Why it is in Polish language? Why not translation mechanism applied here?

@@ -198,7 +198,19 @@
</div>
</div>
</div>

<!-- Export search results -->
Copy link

Choose a reason for hiding this comment

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

Redundant comment

@MockitoSettings(strictness = LENIENT)
public class DashboardExportSearchResultsPageTest {

private DashboardExportSearchResultsPage page;
Copy link

Choose a reason for hiding this comment

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

Why not @Injectmocks if You use already @mock. It is kind of mixed approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants