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

[v1.2] Backup Feature #47

Merged
merged 26 commits into from
Oct 10, 2018
Merged

Conversation

QzSG
Copy link

@QzSG QzSG commented Oct 10, 2018

Relevant #38

QzSG added 26 commits October 3, 2018 16:21
Add backup file path to user preferences model.

Modified equals to compare backup location

Modified string builder to include backup path
Modify ExtraValues json test file
Add `addressBookBackupFilePath` to typical user prefs json test file
[Model]
UserPrefs is required to be in model manager as commands run can affect user preferences which should be part of model.

Refer to original AB4 architecture diagram

Added getUserPref method to Model interface

Fix UserPref model returning wrong path in getAddressBookBackupFilePath

[Tests]
Fix Stubbed ModelManager in AddCommandTest
Make AddressBookStorage and XmlAddressBookStorage have the required backup methods
As long as AddressBook data are equal, userprefs can be different which is okay
Backup Command currently calls model.backupAddressBook, perhaps we should raise a backup request event from command direct and get handled by storage manager directly as we do not need to modify the model
Impossible to make exception to be thrown on linux even with mocking for invalid paths
Create generic online storage module
Implement working backup to github gist
Modify backup command to accept github backup
Copy link

@ChenSongJian ChenSongJian left a comment

Choose a reason for hiding this comment

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

LGTM!

@ChenSongJian ChenSongJian merged commit 8c48fea into CS2113-AY1819S1-T13-1:master Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants