There are a few things I noticed:

1. The text area seems misaligned in the screenshot above.
2. You need to update the db version in glotpress.php otherwise the new table will not be created.
3. There should be a separate button to save the feedback, you shouldn't have to save a translation or a/r/f one to add a note.
4. It might make sense to insert the "by admin @" line inside the note itself when it's saved, otherwise you'll lose any previous time stamps.

It's also throwing two errors with an empty table:

1. Notice: Undefined offset: 0 in C:\wamp64\www\wp\wp-content\plugins\glotpress\gp-templates\helper-functions.php on line 239
2. Notice: Trying to get property of non-object in C:\wamp64\www\wp\wp-content\plugins\glotpress\gp-templates\helper-functions.php on line 226

One other item, I'm not sure I like "feedbacks", perhaps "notes" would be better? Or something else?

Yes of course there few issues but was the first draft but I can fix them asap.
I was thinking that was more helpful to save the feedback when there is the approval to avoid another step.

It's fine to save it during approval, but there should be a button to save it outside of that process as well, at least in my opinion.

I can imagine an approver going back and forth with the translator a few times before getting it just right and have to "reject" it each time seems a little harsh 😉

Good point!
I will work on that asap on all the feedbacks

Something else to think about is creating a "thing" for the "feedbacks" (maybe rename "comments"?) like there is for the translations, projects, etc. (take a look in the gp-includes/things directory).

Done everything, actually wehn you add a new comment at the end of the text in the textarea automatically add the username and the date and not create duplicate in the database.

Actually is missing only the check of the permission on saving.

I don't like the actual workflow, go to the textarea, scroll bottom, add your text and submit and automatically there is the feedback with appended username and date. The comment will be the same entry in the db with only updated the date and the user author of the last edit.

In any case is working and probably will not to be integrated with #702

I'd invert the flow, put new entries at the top.

Perhaps break it up in to two sections, a read only textbox to show the stored notes and a second to write the new addition?

Or perhaps when you click "Add feedback" it gives you a dialog box to enter the new feedback in to?

Uhm yes maybe show the old as not editable text outside the textarea and when saved that content is merged

Perhaps only let admins edit the old text, just in case they need to remove inappropriate content.

It's not clear to me how works the permission system in glotpress so actually I didn't implemented this part to work on the rest of stuff :-)

Permissions are supported through the GP::$permission object (see things/permissions.php for more detail.

Basically you can use user_can() and current_user_can() to test if a user has a specified permission to a specific object type. The permissions are store in the gp_permissions table.

So to edit existing feedback you might do something like:

GP::$permission->current_user_can( 'admin', 'feedbacks', $project_id )

But to simply add feedback you could do something like:

GP::$permission->current_user_can( 'approve', 'translation', $this->id, array( 'translation' => $this ) )

Actions include: read, write, approve, admin, delete. But are not limited to those, any action that can be stored in the permissions table can be referenced by the permission functions.

Likewise for object types.

Ok new code with few bugfix now I will work on the permission part :-)

Permissions implemented, the button label change by permission and enabled to submit both the text if admin.

Would changing the format of the "entries" from "test by user @ time" to:

user @ time added:
text
 

(including the blank line at the end) be more readable?

Likewise I think splitting the button in two makes it easier to understand. One button under the old text when you are an admin to update that and the second button under the new text dedicated to adding new.

With the current structure how do I know what the update button does? If I edit both fields is one going to overwrite the other?

The code check what text is changed and automatically update but I changed the text of feedback as you suggested :-)

For me a single button is more useful because I can maybe change the old notes to remove some of them and add a new one but when I click the button I got a refresh (ajax call that replace all the html of that section) so I can lost all the edits. With a single button the user has the suggestions that can change both of them with a single action and avoid this problem.

I'm not tied to either way, I try and keep buttons doing only one thing as a general rule of thumb though.

The more I look at the code though I'm convinced there needs to be a thing for the table, we don't do direct SQL calls (except in very limited circumstances) as a rule and using an object for the table would keep everything consistant.

Take a look at the gp-includes/things/ directory and the base class in gp-includes/thing.php for how to start.

Two buttons and create a things :-)

TODO: Replace feedback with note in all the code

replaced the reference from feedback to note

I fixed also the code issue in my code using the phpcs ruleset but phpcs report many other issue that are not from my code but in the files that I changed.

It is very appreciated that you are working on this problem 👍
But this solution is very time consuming, PTEs/GTEs are faster, when they correct the error (the translator can compare his and the correctd string to find out, what was wrong).

This solution would be more effective:
#324 (comment)

Not to forget that there is the possibility of bulk edit. An example: The german language has two forms of address: default and formal. Often enough the strings are in wrong form of address ("Sie" in default e. g.). The workflow is: Filter for "Sie", tick all checkboxes at once and reject.

Would be necessary to have also an textarea/reviewers feedback for bulk edit.

@La-Geek thanks for the feedback, the reject with feedback (aka the check boxes) will be looked at after 3.0, for now getting the notes/comments in place is a big step forward.

The current notes implementation isn't just for "reject with comments", but allows notes to be added at any time through the translation cycle. This will allow translators to add information that could be useful in the future as well as to the current translation process.

Closing in favour of the clean git history version PR #975.