Allow uploading attachments to Resume composites

[Gregor Anželj]

Add the option for users to upload attachments to each item of any of the Resume composites (Eduaction history, Employment history, Certifications, Books and Memberships).

[Aaron Wells (u-aaronw)]

This has been uploaded to gerrit in the following patches:

https://reviews.mahara.org/#/c/2137/
https://reviews.mahara.org/#/c/2138/
https://reviews.mahara.org/#/c/2139/

[Aaron Wells (u-aaronw)]

Thanks very much for providing this code, Gregor! :) And I'm sorry it took us so long to get around to reviewing it.

The basic functionality works: I am able to attach files to items in my Achievements, Education, and Employment sections of my resume. These attachments are then listed with download links if I list those resume sections on one of my pages. They also show up under the Content/Files page, in a new directory called "resumefiles".

I do notice a few issues, though:

1. When I'm on the screen that lists my Achievements (or E&E) (i.e. /artefact/resume/employment.php ), and I mouse over the paperclip icon at the top of the column that say how many attachments each item has, the mouse pointer changes to one that indicates it's a clickable link, even though it isn't. (This is probably a CSS styling issue)

2. On that same list screen ( /artefact/resume/employment.php ), when I click on one of my achievements' names to view its description, the list of attachments is also shown, but the attachments are only named, not shown with a clickable link, so I can't tell what's in them easily.

3. The resume item editing screen ( /artefact/resume/editcomposite.php?id=2&artefact=5 ) only lets you add items; it doesn't let you remove them or re-order them, or tell you which ones you've already attached.

4. The mechanism to add files is a simple file upload button, so you can't pick files you've already uploaded into Mahara.

5. Again on the item editing screen, if I've attached a series of files but left out one of the required fields so that the form fails validation, the reloaded form will be lacking all the files I've just attached.

I'm not sure that these problems are all necessarily blockers... as I said before, the code works. It might be worthwhile to take an incremental approach. Which is to say, upstream this code with these non-blocker issues so at least it gets out there, and then come back and fix them as we have the resources to do so.

[Aaron Wells (u-aaronw)]

Items #1 and #2 should be simple fixes, so we can fix those before we release this, along with addressing item #3 by listing the already-attached resume items on the resume item page.

Then we can open a separate bug to address #4, by changing the form from a simple file upload button to the standard Mahara filepicker widget. Doing that will also take care of items #3 and #5.

[Gregor Anželj (gregor-anzelj)]

Aaron hi.

I agree with you that #1 and #2 should be easily fixed.

About #3 and #4:

I've also implemented Mahara filepicker widget for Textbox/Notes
attachments (see: https://reviews.mahara.org/#/c/2029/).
It uses/implements standard Mahara filepicker, but...

...but:
- it worked on previous version of Mahara (1.6.x) and it doesn't work for
1.7.x. I guess there are Javascript issues that I cannot overcome.
- tried to implement this also for E&E etc but they also got a lot of their
internal Javascript (Resume composite stuff) and I don't even know where to
begin to merge/accomodate? Javascript code for Resume composite stuff and
Mahara file picker so there would not be any conflicts and that eveything
would work.

I'm currently just not that good at Javascript. Maybe you could help?

Please see the upper commit link to get the idea. Maybe worth exploring...

HTH, Gregor

2013/5/23 Aaron Wells <email address hidden>

> Thanks very much for providing this code, Gregor! :) And I'm sorry it
> took us so long to get around to reviewing it.
>
> The basic functionality works: I am able to attach files to items in my
> Achievements, Education, and Employment sections of my resume. These
> attachments are then listed with download links if I list those resume
> sections on one of my pages. They also show up under the Content/Files
> page, in a new directory called "resumefiles".
>
> I do notice a few issues, though:
>
> 1. When I'm on the screen that lists my Achievements (or E&E) (i.e.
> /artefact/resume/employment.php ), and I mouse over the paperclip icon
> at the top of the column that say how many attachments each item has,
> the mouse pointer changes to one that indicates it's a clickable link,
> even though it isn't. (This is probably a CSS styling issue)
>
> 2. On that same list screen ( /artefact/resume/employment.php ), when I
> click on one of my achievements' names to view its description, the list
> of attachments is also shown, but the attachments are only named, not
> shown with a clickable link, so I can't tell what's in them easily.
>
> 3. The resume item editing screen (
> /artefact/resume/editcomposite.php?id=2&artefact=5 ) only lets you add
> items; it doesn't let you remove them or re-order them, or tell you
> which ones you've already attached.
>
> 4. The mechanism to add files is a simple file upload button, so you
> can't pick files you've already uploaded into Mahara.
>
> 5. Again on the item editing screen, if I've attached a series of files
> but left out one of the required fields so that the form fails
> validation, the reloaded form will be lacking all the files I've just
> attached.
>
> I'm not sure that these problems are all necessarily blockers... as I
> said before, the code works. It might be worthwhile to take an
> incremental approach. Which is to say, upstream this code with these
> non-blocker issues so at least it gets out there, and then come back and
> fix them as we have the resources to do so.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1103942
>
> Title:
> Allow uploading attachments to Resume composites
>
> Status ...

[Aaron Wells (u-aaronw)]

Hi Gregor,

Sure, I'll give it a try. :)

I'll also take a look at the Textbox/Notes attachment, and maybe I can use that as a starting point.

Cheers,
Aaron

[Aaron Wells (u-aaronw)]

By the way Gregor, where did you get the paperclip icon from? Is it from another open-source project?

[Gregor Anželj (gregor-anzelj)]

The paperclip icon is from free "Farm-Fresh Web Icons" by FreeCow (http://www.fatcow.com/free-icons). I'm not sure if only atribution is enugh. I think they also require a back link to their site?

I guess I forgot that, since I saw "Free" in the title and CC license, so I missed that I need to add back link...

[Gregor Anželj (gregor-anzelj)]

I meant FatCow, not FreeCow in previons comment...

[Aaron Wells (u-aaronw)]

Okay, I'll ask Evonne if she can find or draw us a GPL icon.

[Aaron Wells (u-aaronw)]

Okay, here's a GPL paperclip icon created for us by Evonne Cheung.

[Gregor Anželj (gregor-anzelj)]

Great. Looks nice... :)

[Gregor Anželj (gregor-anzelj)]

https://reviews.mahara.org/#/c/2386/

[Robert Lyon (robertl-9)]

Résumé attachments problems found with currently submitted patch:

Under Content -> Résumé

If I add more than one attachment to a résumé option, say 4, I can delete one or more of them only as long as one is left attached. If I try to remove all 4 attachments and hit save none of them are removed.

When I go to edit the résumé option I get the full 'add a file' options where I can pick from existing uploaded files - this is good - however in the places where I can add a résumé option I get a basic attach file option where I can only upload a file. This needs to be the full choice options like during the the edit state.

Does the Résumé -> Interests part meant to have an attachment option? I do not see one there.

Also now that there are two different displays for resume attachments - some, like Education history, sit in a white box and have attachments as an expandable area - and others like Work skills, where the list is shown by default.

For consistency it is felt that the second type of list should also be an expandable list:

See attached mockup on how it should look/function.

[Gregor Anželj (gregor-anzelj)]

I'll look into adding/deleting all attachments...

About basic atach files for Resume composites (Education, Employment, etc.): I wanted to add full filebrowser alo to add form (not just edit form - as it is now), but discovered that my javascript knowledge is insufficient :(
Could any of you guys do this part?

About Resume->Interests. In the previous version it had the form, since it is a simple resume artefact as goals and skills. Kristina mentioned that goals and skills should have attachemnts. I can add attachments also for interests, but please counsult about this and let me know.

I like the idea od expandaple list (the mockup) and will implement it. Do you have any pointers where to start or how the functionality with expandable arrow works?

[Robert Lyon (robertl-9)]

A question about resume attachments:

When I go to add an attachment to Employment history I get the
  'Attach file Browse option with the [+] ' system

When I edit I get the
  'Attachments Add a file' system

why the two different ways of attaching files?

it would be preferable to have both the adding and editing of resume to use the system that editing is using.

[Gregor Anželj (gregor-anzelj)]

Robert hi.

I answered that question once in the comments.

I would lso prefer the way that is used when editing resume composites, but
I simply can not figure out how to do that, because those forms for adding
resume composites (books, certifications, employment, etc.) are - as far as
I can tell - javascript forms and heavily dependant on javascript and ajax.
I've tried, but failed... Could any of you guys try to do that?

I hope my explanation is clear enough?!

Cheers, Gregor

2013/8/29 Robert Lyon <email address hidden>

> A question about resume attachments:
>
> When I go to add an attachment to Employment history I get the
> 'Attach file Browse option with the [+] ' system
>
> When I edit I get the
> 'Attachments Add a file' system
>
> why the two different ways of attaching files?
>
> it would be preferable to have both the adding and editing of resume to
> use the system that editing is using.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1103942
>
> Title:
> Allow uploading attachments to Resume composites
>
> Status in Mahara ePortfolio:
> In Progress
>
> Bug description:
> Add the option for users to upload attachments to each item of any of
> the Resume composites (Eduaction history, Employment history,
> Certifications, Books and Memberships).
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mahara/+bug/1103942/+subscriptions
>

--
----------
Gregor Anželj, prof.
Gimnazija Bežigrad, Ljubljana

[Robert Lyon (robertl-9)]

With the 'Add' attachment option - trying to get the 'filebrowser' artefact chooser will not work at the moment.

The problem is the file upload part of the chooser - especially the 'drop files here' bit - as is not designed to be added more than once to a page.

So possible options going forward are:
1) make the filebrowser artefact chooser upload part work for multiple instances on a page, or
2) go with what is there currently
3) make the forms appear on separate pages like how the 'edit' item works

[shanen (shane-nuessler-canberra)]

This is great news, thanks a lot!

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/2530
Committed: http://gitorious.org/mahara/mahara/commit/d88b94001ec9980536f278cadd5fe827e32f95cb
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit d88b94001ec9980536f278cadd5fe827e32f95cb
Author: Aaron Wells <email address hidden>
Date: Thu Sep 26 17:05:48 2013 +1200

Undoing changes to the ArtefactTypeResumeComposite class signature

Bug 1103942: Resume attachments included two unnecessary changes to
the ArtefactTypeResumeComposite class which will break dependent code.

First, it added a new abstract function. This will cause any file
that contains a subclass that hasn't implemented that abstract function
to error out. So I'm providing a sensible default implementation
instead of making the function abstract.

Second, it adds a Pieform as the first parameter to ensure_composite()
to make it print Pieform errors. New parameters should always be added
as the last parameter to a function, with a default value, so that
existing calls to the method will still work. Additionally, there's
no need for the Pieforms logic to be in that call, since this function
is often used in a non-Pieforms context. I've refactored out the need
for that parameter.

Change-Id: I48b1f2375def6ed6aafdac389fcbb120ad04d359
Signed-off-by: Aaron Wells <email address hidden>