-
Notifications
You must be signed in to change notification settings - Fork 411
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
[CS2103T-F11-3] SecureIT #62
base: master
Are you sure you want to change the base?
[CS2103T-F11-3] SecureIT #62
Conversation
* is reasonably comfortable using CLI apps | ||
|
||
*Value proposition*: manage contacts faster than a typical mouse/GUI driven app | ||
|
||
*Value proposition*: Remember only one password, and save the hassle of remembering all other confidential documents (account details, credit card details, secret files, secret notes). Have a safe and secure way to store all confidential documents locally, without the use of the online/ third party / cloud-reliant vaults. | ||
|
||
[appendix] | ||
== User Stories |
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.
Our team has read through your user stories and we notice that some important ones might be missing.
Must have:
- List passwords
- Find passwords associated with some accounts
- List credit card info
- Add new file to encryption system
Nice to have:
- Two-factor authentication
@@ -295,56 +301,479 @@ Priorities: High (must have) - `* * \*`, Medium (nice to have) - `* \*`, Low (un | |||
[width="59%",cols="22%,<23%,<25%,<30%",options="header",] | |||
|======================================================================= | |||
|Priority |As a ... |I want to ... |So that I can... |
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.
Some of your user stories are in the wrong format, e.g. I can be sure ...
.
|
||
Reliability | ||
|
||
* The app shall throw appropriate exceptions when any user input is invalid or any user action fails to execute completely. |
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 it would be better if you could paraphrase this into the system has to be robust to errors
, because an NFR should not be too detail.
|
||
Data Integrity | ||
|
||
* Should be able to check for the data integrity as to verify that no one has modified the files within secureIT in an unauthorised fashion. |
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.
This sounds too functional, maybe it would be better to rephrase the sentence into this program should consider data integrity of files
.
--- | ||
|
||
[discrete] | ||
=== UC21 - Encrypt a file |
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.
Could be missing extension, 1a where file to encrypt is missing
* 2a. Details entered are invalid | ||
+ | ||
[none] | ||
** 2a1. FileSys shows an error message |
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.
Could be an unnecessary UI detail mentioned might want to try: "notify user" instead of "shows error message" which might be a bit specific
|
||
--- | ||
[discrete] | ||
=== UC11 - Add a password |
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.
Might be missing a guarantee: "password is added"
@@ -98,9 +93,9 @@ image::LogicClassDiagram.png[] | |||
*API* : |
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.
@@ -109,7 +104,7 @@ Given below is the Sequence Diagram for interactions within the `Logic` componen | |||
.Interactions Inside the Logic Component for the `delete 1` Command | |||
image::DeleteSequenceDiagram.png[] | |||
|
|||
NOTE: The lifeline for `DeleteCommandParser` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram. | |||
NOTE: The lifeline for `DeleteNoteCommandParser` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram. | |||
|
|||
[[Design-Model]] | |||
=== Model component |
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.
|
||
[NOTE] | ||
If the `currentStatePointer` is at index 0, pointing to the initial address book state, then there are no previous address book states to restore. The `undo` command uses `Model#canUndoAddressBook()` to check if this is the case. If so, it will return an error to the user rather than attempting to perform the undo. | ||
image::AnalysePasswordSequenceDiagram.png[] |
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.
docs/DeveloperGuide.adoc
Outdated
The undo/redo mechanism is facilitated by `VersionedAddressBook`. | ||
It extends `AddressBook` with an undo/redo history, stored internally as an `addressBookStateList` and `currentStatePointer`. | ||
Additionally, it implements the following operations: | ||
image::AnalyserClassDiagram.png[] |
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 figure caption seems to be missing for a few diagrams.
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.
Some changes might help to improve the document.
@@ -98,9 +93,9 @@ image::LogicClassDiagram.png[] | |||
*API* : | |||
link:{repoURL}/src/main/java/seedu/address/logic/Logic.java[`Logic.java`] |
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.
Diagram can be broken down into smaller parts
@@ -109,7 +104,7 @@ Given below is the Sequence Diagram for interactions within the `Logic` componen | |||
.Interactions Inside the Logic Component for the `delete 1` Command | |||
image::DeleteSequenceDiagram.png[] | |||
|
|||
NOTE: The lifeline for `DeleteCommandParser` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram. | |||
NOTE: The lifeline for `DeleteNoteCommandParser` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram. | |||
|
|||
[[Design-Model]] | |||
=== Model component |
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.
The diagram shows a good overview of all the components, however it is a bit too small. Maybe it could be broken into smaller components.
docs/DeveloperGuide.adoc
Outdated
@@ -14,7 +14,7 @@ ifdef::env-github[] | |||
endif::[] | |||
:repoURL: https://github.com/se-edu/addressbook-level3/tree/master | |||
|
|||
By: `Team SE-EDU` Since: `Jun 2016` Licence: `MIT` | |||
By: `SecureIT` Since: `Jun 2016` Licence: `MIT` |
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.
The date appears incorrect.
@@ -109,7 +104,7 @@ Given below is the Sequence Diagram for interactions within the `Logic` componen | |||
.Interactions Inside the Logic Component for the `delete 1` Command | |||
image::DeleteSequenceDiagram.png[] |
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.
The diagram shows AddressBookParser
. Also, since there are multiple directories, the context of delete
is not explicitly mentioned in the preface. There is also no description to describe the diagram.
docs/DeveloperGuide.adoc
Outdated
The undo/redo mechanism is facilitated by `VersionedAddressBook`. | ||
It extends `AddressBook` with an undo/redo history, stored internally as an `addressBookStateList` and `currentStatePointer`. | ||
Additionally, it implements the following operations: | ||
image::AnalyserClassDiagram.png[] |
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.
The diagram can be resized.
It appears too verbose.
Each component could be described individually.
Missing figure caption.
docs/DeveloperGuide.adoc
Outdated
The undo/redo mechanism is facilitated by `VersionedAddressBook`. | ||
It extends `AddressBook` with an undo/redo history, stored internally as an `addressBookStateList` and `currentStatePointer`. | ||
Additionally, it implements the following operations: | ||
image::AnalyserClassDiagram.png[] |
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.
The Match
direction seems incorrect, did you mean Match
implements BaseMatch and DictionaryMatch extends BaseMatch?
docs/DeveloperGuide.adoc
Outdated
* **Alternative 2:** Individual command knows how to undo/redo by itself. | ||
** Pros: Will use less memory (e.g. for `delete`, just save the person being deleted). | ||
** Cons: We must ensure that the implementation of each individual command are correct. | ||
image::InitPasswordSequenceDiagram.png[] |
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.
There are missing activation bars for Storage, Model, Logic and Ui after they are constructed.
* **Alternative 2:** Individual command knows how to undo/redo by itself. | ||
** Pros: Will use less memory (e.g. for `delete`, just save the person being deleted). | ||
** Cons: We must ensure that the implementation of each individual command are correct. | ||
image::InitPasswordSequenceDiagram.png[] | ||
|
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.
We think it would be helpful to have an activity diagram to summarise the feature.
Fix password branch
Fix file duplication when file is being opened before encryption
…rd-2 Fix analyse password 2
Merge branch 'update-ug' of https://github.com/AY1920S1-CS2103T-F11-3/main into update-ug
Fix notes bugs
…ension Change the rename file command to retain file extension
…-util Add explanation of encryption method in the developer guide
Add new line to one command
… into add-notes-test
Add notes test
Do final edit on the user guide
Final tweaks to PPP
Change version number to v1.4
Rename img file
@jityong
@niqiukun
@yhtingg
@eejian97