Feature/detailed review attachments view

Reviewers still needed a more detailed attachments view, for example to display attachment names and comments. This PR should fill that gap.

Additionally I continued working on the comments from #736, which I will further address here.

I have tried to reduce the number of unique classes, however I have failed :-/

Items vs. Slots

As these classes look rather similar and do similar things, it might make sense to just use give slots the right methods and use slots everywhere. The main problem with this is that an item in the documents list does not always represent an attachment. Sometimes it might be the PDF application, METC decision, or what have you. Those files do not live in a slot.

This is why the item class needs to exist as a wrapper around whatever thing is being put in the list.

Slots and items also have different purposes. Slots really are a stepper object. They represent a place to put an attachment, even if it's empty. And the DocItem (and the AttachmentItem wrapper) should represent any kind of document in a list.

Vision

So then when it comes to vision, I guess that I like having small wrapper classes because they make our code less interdependent. This makes it easier to change things down the road, and will hopefully also avoid us having to fix a million things at once in the future.

For example, by having made these kinds of container classes like DocItem when developing the previous documents_list, it was very easy to reuse code for the newer attachments list. The following class does seem a bit frivolous:

class AttachmentItem(DocItem):
    """
    A wrapper for AttachmentsList items that works with Attachment slots.
    """

    @property
    def attachment(
        self,
    ):
        return self.slot.attachment.get_correct_submodel()

    def get_link_url(
        self,
    ):
        return self.slot.attachment.get_download_url(
            self.slot.get_proposal(),
        )

    def get_filename(
        self,
    ):
        return self.attachment.upload.original_filename

But I really like that with just a few glue methods we can reuse existing code. Slots and items serve only slightly different purposes, but I still prefer to have them each be good at their own jobs and then write a bit of glue inbetween.

Other strong similarities / potential merges ion the future

There are strong similarities in how we fetch and sort attachments per attached object in the attachments list and in ProposalAttachmentsList.get_context_data(). That's kind of lame but I'm not yet convinced it's worth mashing these together.

I do think it's kind of attractive to mangle DocList into the stepper. It would be nice to have stepper.slots.per_item and stepper.slots.as_containers functions which we could use in many places. However, because we would still need to somehow shoehorn things like the METC decision into the review sidebar, I have left this as is for now. It's just not worth the effort as things are still developing. But it might be worth doing in the future.

Editing attachments as secretary

This is more of an emergency feature, hopefully it will be far less necessary now that the attachments system is improved. Because it should only be used as an exception, I'm fine with it being the same as the user's edit attachments view.

I also believe that it is good to only have one view to maintain, and it guarantees that there's no mismatch between the slots that the secretary sees and the slots that user sees.

It would be an improvement to remove the stepper from this page. But I just don't really see it as worth the effort right now.

Conclusion

I have moved some methods such as comparable() and is_new() from the Item to the Slot class, but in general I have kept the number of classes the same.
Wherever I have failed to reduce the number of classes, I have rearranged them in more logical places which will make things more maintainable going forward. For example in their own reviews/utils/attachment_utils.py
Of course there's also a new detailed attachments view, check it out!

DH-IT-Portal-Development / ethics