Link attachments to notes in database

[vboctor]

This is work in progress that is published to get feedback.

The change in db is to add a nullable bugnote_id (unsigned integer) to the bug file table with default value of 0.

• 0: old file uploads - no explicit association.
• null: file explicitly associated with the issue (not notes).
• non-zero: file explicitly associated with an issue note.

Fixes the following:

• 21733: Attachments should be linkable to notes in db
• 24577: When deleting a note made of 2 images (attachments) + text, delete operation has to be requested 3 times, not one
• 25935: Warning for users when making public notes with attachments private
• 9802: Private attachments
• 22817: "private bugnotes" as default setting prevents uploading further attachments
• 25960: Add files information to EVENT_BUGNOTE_ADD event

Features:

• It is now possible to upload attachments to private notes.
• Newly submitted notes with attachments are explicitly linked in the db.
• Deleting a note deletes associated attachments (explicit or implicit).
• Switching a note to private/public converts implicit links to explicit links and marks the note and its associated attachments as private/public.
• History entry related to attachments now include the bugnote id that the attachment is associated with. This is used to determine visibility of the history entry. Not currently visible in UI.
• The history entries related to adding files to notes are hidden if user doesn't have access to associated notes and files.
• If a note and its associated files is deleted, then the note add and delete will remain in history, but file add/remove will not longer be visible if user doesn't have access to private notes (since there is no way to know which notes this file was associated with and its private/public status).
• Support submitting private attachments without a note. An empty note is created and files are associated with it. This also enables changing from private to public and vice versa.

Not Supported:

• Upgrading the db to convert all implicit links between notes and attachments (based on submission time with notes) into explicit links. Such conversion happens when user takes specific actions notes with implicit attachments.

[fgarciapatino]

Hello.

I have just finished a work for all these fixes and features, that also supports the first "not supported" item. A note with a generic text is created when the user send files from the issue creation, and the files are linked to it. Why let this functionallity orphan?

I do not need an update strategy because my project was a new mantis installation, but I would suggest a procedure based in the current algorithm at bugnote_view_inc executed once.

I test the following use cases and works fine:

• Project deletion
• Issue creation with attached files
• Issue deletion
• Note addition
• Note deletion
• Note moved
• Note cloned

I also test the access level behaviour and also works fine.

I could send you all changes I made, but my strategy is different, not extending the existing {bug_file} table, and creating a note_file entity instead to store the note/file relationship. Of course I think this is a better and clean approach, and has a right semantic. You cold save a lot of work because you only have to test the proposed solution, and re-order things if you consider a function missplaced, or the opportune revission of code practices (I tried to follow).

Unfortunately, the solution drags some unuseful parameters, included for compatibility, or for re-using already working and tested code because was free.

The changes I made are related to the following files (same you will have to modify):

• Database: {note_file} table created to store bugnote / file relationships
• File: IssueNoteAddCommand.php:
• File: bug_activity_api.php
• File: bugnote_api.php
• File: bugnote_add.php
• File: bugnote_add_inc.php
• File: bugnote_view_inc.php
• File: file_api.php
• File: IssueAddCommand.php
• File: common.js
• File: print_api.php:
• Resource: custom_strings.php (config)

From a technical point of view I found some reasons to prefer the {note_file} table approach, but most important is not to duplicate and mantain an already done work.

[cproensa]

I found some reasons to prefer the {note_file} table approach,

I understand that you prefer normalization of the relations, but i suggest that the roadmap for the future to be that files should be decoupled from issues. We'd have then an independent file storage, and relations for each file to specific activities: notes and/or issues, but not limited to those types (think other contexts where files may be useful)
That is a deeper change, so for now, i think i'm fine with Victor's approach.

Haven't tested this PR yet, so i'm just commenting on the schema topics.

[fgarciapatino]

That's exactly what happens when you use the bug_file table as an entity, and when you decide to create a new bugnote_file table to store file and bug note relationship. In my implementation, I have not thange the bug_file table, but files are not managed anymore as a bug complex type. What you say is what I have implemented. El jue., 25 jul. 2019 20:20, Carlos <notifications@github.com> escribió:

…

I found some reasons to prefer the {note_file} table approach, I understand that you prefer normalization of the relations, but i suggest that the roadmap for the future to be that files should be decoupled from issues. We'd have then an independent file storage, and relations for each file to specific activities: notes and/or issues, but not limited to those types (think other contexts where files may be useful) That is a deeper change, so for now, i think i'm fine with Victor's approach. Haven't tested this PR yet, so i'm just commenting on the schema topics. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1529?email_source=notifications&email_token=ABE7GHRBB67OQHQ5KQCWDKDQBHVGHA5CNFSM4IFVACXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22LDLI#issuecomment-515158445>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABE7GHSU5QF24UALO2N6JDDQBHVGHANCNFSM4IFVACXA> .

[fgarciapatino]

It has no sense to say that files should be decoupled from issues, but the solution was to include a note attribute in the file table. Doesn't it?

In my implementation, the bug_file still exists because of my own unknowledge reasons, but in my model, it is not used as a bug collection and the table should be renamed to "file", and it is an independant entity which you can link with whatever business entity you want. This is a typical ORM discussion.

Now, "be a file" is just a format specialization, instead of a specialized bug attribute, as well as you suggested long time ago, and as a result, messages (or activities, or notes...) can have text, and/or file attachments as a content.

[vboctor]

@fgarciapatino

I have just finished a work for all these fixes and features, that also supports the first "not supported" item. A note with a generic text is created when the user send files from the issue creation, and the files are linked to it. Why let this functionallity orphan?

This can easily be added on top of this change by allowing notes to be empty if they have associated file and when rendering we can show a localized message indicating that there is no text is associated with the note or just not render anything.

note_file vs. re-using existing table

I have opted for the smallest change to address the main critical limitations we have today. My solution also acknowledges that the current table already has bug note files that are not explicitly linked.

@cproensa thanks for the review.

@dregad @atrol I'm planning to check this in after cutting the next release.

[vboctor]

@crosoclipr

Added "Support submitting private attachments without a note. An empty note is created and files are associated with it. This also enables changing from private to public and vice versa."

I believe it is the best solution to have all issue related attachments in the current bug_file table. This also keeps attachments that are explicitly linked and ones that are implicitly linked in the same table and keeps the code and changes much simpler. We can evolve the rationalization of the schema in the future and debate the pros/cons of different normalization models. However, I'm leaning when we head there to have a single attachments table that can be associated with different entities.

Note also project files is deprecated or at least de-emphasized.

[fgarciapatino]

In my implementation, the file table is a master table which spreads references where needed. Same strategy used for bugnote text or bug text. This let you to decide to implement collections or attributes to other entities in a clean way. Also, this allows to re-use (or not) files, with a clear treatment. El sáb., 3 ago. 2019 22:56, crosoclipr <notifications@github.com> escribió:

…

Hello, Please consider adding this as a notes_file table and not to the existing bug file table. History we have project_file etc - this would be in keeping with what others have said and the existing structure. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1529?email_source=notifications&email_token=ABE7GHSLM77UZR3IAPOTLYDQCXWILA5CNFSM4IFVACXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3PVUXI#issuecomment-517954141>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABE7GHQMHL7L4V56P5TRYZDQCXWILANCNFSM4IFVACXA> .

[fgarciapatino]

For this concrete case, the model has a short discussion taking in account the goal. You only have to ask yourself "what is a file for?". Files has no sense by itself for the business, and needs to be related to something else (bug notes, projects, events...) in the same way you accept text as an entity for bugnotes or bugs, and in fact, the "shomething else" could be whichever other class.

Now, files are just specialized collections of bugs with a diablical logic to be rendered in the right order, and if you add a bugnote_id column to the file table, you are not changing the concept. It is true you are simplifying and cleaning some dirty things, but not having a real structural change in terms of decoupling files. Imagine you prefer to re-use references of files instead of making a copy of them each time.

If you use the bug_file table as a master table (without changes), and create a new one (bugnote_file) simply to store the relationship, you solve the same problem without conflicts neither with the evolving strategy nor the current implementation (same impact you had to develop your solution), so it seems a higher solution.

By the way, what are the known "main critical limitations we have today"?

[dregad]

I have not analyzed this in detail, but I'm wondering if BugnoteData structure should be updated to reflect the addtion of attachments.

[vboctor]

@dregad Since the cardinality of bugnote to attachments to BugnoteData is 1:n and not all attachments are linked explicitly, I didn't add attachments to BugnoteData. We can always revisit in the future.

[priocomdevops]

Much needed changes, thank you.
Have they been included in Mantis 2.25?
Kind regards.