refactored dashboard ExportSearchResultsPage #2611
Conversation
lbownik
force-pushed
the
dashboard_refactoring
branch
from
1be03d6 to
add4bee
Compare
| @@ -15,6 +15,8 @@ | |||
|
|
|||
| import static java.util.Optional.ofNullable; | |||
|
|
|||
| import java.util.Collection; | |||
| @@ -94,6 +94,10 @@ public String getLocaleTitle() { | |||
| public int getFilesPerPage() { | |||
| return filesPerPage; | |||
| } | |||
|
|
|||
| public boolean canEditDashboard() { | |||
There was a problem hiding this comment.
Should be moved to the LOGIC section below.
| @Mock | ||
| private DataverseSession session; | ||
| @Mock | ||
| private SystemConfig systemConfig; | ||
| @Mock | ||
| private DatasetFieldTypeRepository datasetFiledTypeRepo; | ||
| @Mock | ||
| private PermissionsWrapper permissionsWrapper; |
There was a problem hiding this comment.
You still need a NavigationWrapper mock.
There was a problem hiding this comment.
I dont need it since I don't test DashboardExportSearchResultsPage.verifyAccess() method, and I don't intend to for now.
There was a problem hiding this comment.
But you're purposefully breaking the constructor interface by providing null and you're still creating a SystemConfig mock even though it doesn't seem necessary. I mean, it's not very consistent. Tests are in a way much like production code.
| .thenReturn(asList(this.type1, this.type2)); | ||
| when(this.datasetFiledTypeRepo.findAll()).thenReturn(types); | ||
|
|
||
| this.page = new DashboardExportSearchResultsPage(this.session, null, |
There was a problem hiding this comment.
Why did you remove @InjectMocks?
There was a problem hiding this comment.
the constructor of DashboardExportSearchResultsPage invokes datasetFiledTypeRepo.findAll().
i need to call when(this.datasetFiledTypeRepo.findAll()).thenReturn(types); BEFORE instantiating DashboardExportSearchResultsPage
otherwise datasetFiledTypeRepo.findAll() returns empty list
There was a problem hiding this comment.
I guess the problem stems from using logic inside the constructor, which isn't always the best choice (specifically in a dependency injection context). For instance, you could have extracted the logic to a @PostCostruct annotated method and call that method here to make the intend (logic usage) more obvious.
refactored dashboard ExportSearchResultsPage