Profile Picture for each user

[tshradheya]

Hey guys,

For V1.2 I implemented the Display Picture command to add/remove/update display picture for existing command.

I know a few more people who have been working on it/implemented partially and are struggling with few problems such as:

• Not able to store image in directory so as to make the display picture permanent
• Naming of pictures so as to not allow any replacement ( Tip: Use unique and predictable hashcode)
• Not able to bind the ImageView so that it instantly refreshes
• Command not working on Mac/Windows
• Having a lot of restrictions like e.g. picture should be saved on the desktop, the image should be of a particular format.

I have tried to take care of all of these, although there still might be bugs I haven't encountered yet. Let me know if you find any bugs

My documentation for this can be found in the following PR I have mentioned some alternatives which I really wanted to implement but had to make a decision to choose the current one due to design and requirements.

The actual implementation is in this PR

Also many faced problems with Test coverage falling ( just like me), so I tried my best to minimise it by currently(-0.3%) by adding more tests for all cases. I am still working on some GUI tests although it is very confusing. Some tests are in this PR

Hope you guys find this useful.

P.S. I still have to clean up the code a little so sorry for those small errors in coding practices.

EDIT: So just today I realised my implementation doesn't work on jar files as my tutor told me its not possible to change .jar file contents. So I have fixed this bug and added tests for it in this PR

[teclu]

I haven't made my PR yet, but I'm finalising my implementation for a custom profile picture for each contact too that users can import by an online URL!

However, the way I implemented it is somewhat different. Instead of saving the picture into a local directory, I was able to store it inside "DisplayPicture" by saving the URL and Image (mine's called Avatar). As of now, I am able to store the picture in AddressBook (upon closing and reopening, it's still there). I don't think my code is that ideal, but I hope my idea could help you out in someway!

I'll get my PR tomorrow up tomorrow and make a new issue in the forum for sharing (and link it back to yours so that people can look at both ways of doing it).

Thanks for sharing your implementation! I'll credit you if I choose to adapt anything from it. 😁

EDIT: Running into Travis CI problems, so I don't think I can get it up so quickly. T-T

EDIT 2: Got it to pass Travis and Appveyor. You can check out my PR here. No guarantees that it's bug free though.

[tshradheya]

Ohh that's great.

The reason I stored in my directory is because I ask the user to add the image from his local device. I had considered your idea of implementation but that would only work for images stored online.

The reason I didn't go ahead with that was because:

• The online URL for an image could change over time causing bugs in the product.
• I want to allow the user to add images from either local device/online source, so for my current implementation, I decided to store the image somewhere as then I can easily extend it to online images by storing those images in the same place in directory. ( OCP :P).

In case I have any problems with online images, I will credit you if, I decide to take implement you style for it.

Thanks

[tshradheya]

@teclu Just saw your PR. Looks good.

However I noticed that you didn't do any testing for 'Avatar' class. And from my bad :( experiences, I assume your code might not work if you run through jar folder. (not really sure).

The problem is because the directory in jar is different(it is technically a zipped file). So your src/main/java/model.... might not be correctly referenced as the path isn't exactly same.

Solution:

• Store in resources folder and use getResoucresAsStream() to read the images. OR
• Using preferences.json create a new folder to store images in the same directory as the jar file. ( I used this method as I store the image given by user so he can delete his local copy if he/she wants to)

P.S. I would suggest going for the solution only if you are ready for a long night of coding. I spent 20+ hours just to fix the problem as only later I realised my code doesn't work using jar file. :P

[teclu]

@tshradheya I'll probably be adding testing for my Avatar class tomorrow (or for v1.4). I haven't tried running the program in the .jar folder, but I'll let you know if it works. If it doesn't, then I'll probably follow your solid suggestion. 😁

EDIT: After building the .jar file and testing, I can confirm that my implementation works, but as of now it does not support saving the image (so your implementation is better).

Also, I can confirm that both valid online URLs and local directory paths for images will work in my implementation, so as long as these paths remain valid and do not change.

[aali195]

Thank you for posting this. I have been attempting to implement something very similar to this for the past week but I was too ambitious with that idea as I'm still too much of a beginner when it comes to programming (ended up missing the last milestone). I didn't want to give up on the feature after all the time I spent on it.

The current implementation is very similar to what I first had in mind except for the hashcode file name. Grab the filepath from the command line, save the picture somewhere permanent and display it etc. before deciding to go with the idea of using base64 encoding of the images to store into the actual addressbook which I couldn't manage

I will reference this post in my docs and I will let you know if I run into any bugs.

[karrui]

After 1000000 hours of trying to fix Travis failing due to OS specific filepaths, I feel comfortable enough to share my own implementation. Seems like there's a fair bit of difference in the way things are implemented.

My implementation only allows the user to set a profile picture by inputting an image URL: sa 1 sa/http://www.example.com/image.jpg, which the parser then creates a SetAvatarCommand which creates an Avatar object.

The Avatar object will invoke the ProcessImageFromUrlToFileForAvatar class which I've implemented to convert the URL to an image file located in the local disk.

You can also clear the avatar with empty parameters, which will then cause the file to be deleted

Some tiny issues remain -- if the user deletes the whole avatar folder, the application will fail to load due to IllegalValuesException as the application would not be able to load the addressbook.xml properly. Will see if I am able to fix this before v1.5, but I am pretty burnt out right now, hah!

Here is my PR, where you can laugh at or cry at the amount of CI failures, urgh!

EDIT: Note that there is a later PR that fixes a couple of bugs that changes SetAvatarCommand to be a normal Command instead of an UndoableCommand, see https://github.com/CS2103AUG2017-F11-B3/main/pull/89

[tshradheya]

@karrui Haha the struggle is real.

After all those hours of debugging and having to wait for Travis patiently, just feels like to drop out of college and sleep peacefully without any work. :P

On a serious note, just saw your PR, somethings are similar and some are very different. I like how you took care of most of the problems which arose due to getting images from online links.

Take day off from 2103 after this cause that was the only way I could relax myself after those long hours of coding and debugging.

[tshradheya]

@aali195 Just had a look at your PR. Very similar implementation.

However, I would like to point out that the method call of run of ImageUtil.java is actually something which goes against the Law of Demeter and is not a good Software Engineering Practise.

I am assuming you had a look at my PR and followed a similar strategy. Since I have implemented the same way, I myself am planning to do the reading and saving in Storage component to maintain consistency. Since I am working on other feature for this milestone, I will be doing my code clean up and improving implementation later on. I will be referencing the PR I create to ensure SEP is followed in by a week or two.

Anyhow, great work on getting the image feature to get working.

[tshradheya]

As mentioned before that my implementation is not exactly correct in terms of Software Engineering principles. So for this week along with implementing a new feature I also enhanced the architectural design for my profile picture feature to make it better.

Here is the PR in which I changed the architecture and have added tests accordingly.

P.S my PR might not be clean as I added some other things as part of fixing bugs. Most of the changes are in Storage directory where you can have a look to enhance your design as well.

Also, a bug still exists that the image stored doesn't delete automatically if a person is deleted. I have to fix this bug. Even though its just a function and an event handler I am pretty tired of enhancing this feature so I will be doing it in V1.5rc along with making sure this works well for sharing of contacts as well and write update the new PR here.

[karrui]

📢 IMPORTANT: FIXED BUG IN MY IMPLEMENTATION

For anyone who is using my implementation, take note that I have changed my command to be NOT undoable, as I discovered a bug -- deleting an avatar cannot be undone regardless as File.deleteOnExit() cannot be undone, and will cause an application crash the next time the application tries to load addressbook.xml. It has been changed to be a normal Command, and the user will have to readd the avatar manually.

However, there is still a check when overwriting an existing avatar with a new avatar (can be the same URL, doesn't matter) -- the relevant method will still delete the current avatar before adding the new one, keeping AvatarStorage clean from unused avatars that were stored but overwritten with a new avatar by the user using the command.

The relevant code updated can be found in https://github.com/CS2103AUG2017-F11-B3/main/pull/89