-
Notifications
You must be signed in to change notification settings - Fork 0
admin ui MVC set up #5
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things to check before committing, do we still need any commented code?
databaseStatus.setName("Last Backup"); | ||
databaseStatus.setValue(updated_date); | ||
databaseStatusRepository.update(databaseStatus); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding the date in two places was a backup thing?
|
||
public class IndexViewModelPost { | ||
//network statuses | ||
/* private String networkStatus; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't need this anymore we should delete
this.messages = new ArrayList<>(); | ||
|
||
if (databaseStatusesResponse.hasErrors()) { | ||
Logger.error("UpdatesController-databasePost()","Failed to update statuses"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably have a clearer error string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some name changes. Looks good!
List<? extends IDatabaseStatus> kitStatuses = databaseStatusRepository.findAll(DatabaseStatus.class); | ||
response.setResponseObject(kitStatuses); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t this variable be named databaseStatuses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sir, gotta change this... done!
ds.setValue("connected"); | ||
assertEquals("connected", ds.getValue()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the tests look good, they test exactly what we need right now
/*List<? extends ISystemSetting> allSystemSettings = systemSettingRepository.findAll(SystemSetting.class); | ||
|
||
try { | ||
if(systemSettings == null){ | ||
//If systemSettings is null, that means that all settings buttons were unchecked. | ||
for (ISystemSetting ss: allSystemSettings){ | ||
ss.setActive(false); | ||
systemSettingRepository.update(ss); | ||
} | ||
} else { | ||
for (ISystemSetting ss : allSystemSettings) { | ||
if (systemSettings.contains(ss.getName())) { | ||
ss.setActive(true); | ||
systemSettingRepository.update(ss); | ||
} else { | ||
ss.setActive(false); | ||
systemSettingRepository.update(ss); | ||
} | ||
} | ||
response.setResponseObject(allSystemSettings); | ||
} | ||
} catch (Exception ex) { | ||
response.addError("", ex.getMessage()); | ||
}*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove commented out code if no longer needed
|
||
ServiceResponse<List<? extends IKitStatus>> kitStatusesResponse = updatesService.retrieveKitStatuses(); | ||
if (kitStatusesResponse.hasErrors()) { | ||
throw new RuntimeException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe log error details?
Admin UI MVC setup