Global headers

[alevalv]

This PR adds Global Headers and removes two dialogs, one when a request fails and one when the project is saved successfully.

Global Headers are defined at the project level and are shared by all existing Request under the project. They can be overwritten by the request if they have the same key.

This pull request also include a refactor of the projectproperties ui, adding tabs for better navigation.

[fabiobento512]

Thanks for the pull request. I will try to review it in this weekend.

[fabiobento512]

I just had a quick look, some comments / questions:

1. I think that feature may be interesting, even though default headers can be used to accomplish something similar. Though to accept this idea I think we need to have an option, per request, to disable the global headers for that specific request. Something like this:

We need to think how this feature may affect all users, and right now I don't think there is a way to disable global headers for a specific request if those are enabled. This may have an unexpected effect for some users requests. I know that we can override them (making them empty for instance) but that is not the same as not sending them,

2. I would rather see the global headers also showing in the requests themselves, like any other header. But instead these should be disabled (to visually indicate that those are global variables) and should not be possible to remove, unless overridden. Like this:

Additionally we can also add a new column to the table indicating if it is a global header, but this may not be necessary.

3. The authentication tab in the project properties is messed up from the previous version. There's some problem in the layout there I believe.

4. Since this a new feature, more specifically in the project file itself, the ProjectFileFRequest::upgradeProjectFileIfNecessary(const QString &filePath) function (projectfilefrequest.cpp) needs to be updated so you can include a upgrade step from version 1.1c to 1.2. This upgrade should create an empty Headers element, so when you access it as pugi::xml_node headers = doc.select_node("/FRequestProject/Headers").node(); no problem arises (and also for consistency, since request with no headers right now generate a empty Headers element as well). I would also rather call this element GlobalHeaders instead of just Headers, since it is easier to distinguish from the requests headers itself (both in the code and in the xml file, for debugging).

5. Regarding the removal of the dialogs, I am ok with the removal of the one where the request fails, since the user can always check the return code and description message. Although about the one saying that the project properties were saved correctly I would like to keep it, so the user knows that the file has been saved by the program at that point (no need for extra saving from the user side).

I will review the code itself once we have addressed these points.

[alevalv]

Sorry for the delay, I've been really busy these last days, I have updated the PR to address your comments.

GlobalHeaders will be displayed on the header table but they are marked as not modificable, and will never be. If the user wants to override one of them they should add a new one to the request. This was the less complex solution that I found that allow FRequest to show the global headers.

I added a new configuration to hide the project saved successful dialog (I feel that is useful for me, so I would like to keep it), by default it is enabled as it was before.

There is an issue with the new checkbox to disable the globalheaders, mainly on refreshing the UI to show/hide the headers on the table.

[fabiobento512]

Hi @alevalv , thanks for applying the changes. It is not a problem, I also have been busy myself. I will check everything again and I will report back if necessary.

[fabiobento512]

I am creating some commits to finish your feature. I am reviewing all the code and making adjustments where I think it is necessary, I will report here the progress. In the end I will ask you to do a final (or not) review.

Question/Comment 1 has been reviewed. Question/Comment 4 has been reviewed.

[fabiobento512]

Question/Comment 3 has been reviewed. Question/Comment 5 has been reviewed.

[fabiobento512]

Sorry for the massive delay. I have been very busy. I will finish this review in next 1 / 2 weeks.

[fabiobento512]

Question/Comment 2 has been reviewed.

I submitted a couple of changes to this branch. I fixed some things and decided to remove the override feature of the global headers (if you add one with the same name).

The reason for the last one, is that it seems that it is possible to have multiple headers with the same key: https://stackoverflow.com/a/4371395

and this would make very hard to track which header would be overriden.

If we add scripting to FRequest, the functionality can be improved using user scripts.

@alevalv Please check the changes that I did, and if you agree we can push this to master, so the functionality gets add to the next release.

[alevalv]

I didn't know about having the same header multiple times. The checkbox override is good enough for enabling/disabling/overriding so I'm ok with that change @fabiobento512 .

I was able to test the scenarios related with globalHeaders with success, but I wasn't able to send a request with multiple headers with the same key. I might be having some issues with my environment or it could be qt issue, but the FRequest code is working as expected.

[fabiobento512]

@alevalv You are right. I tried send duplicate headers and I checked the traffic in wireshark, for some reason FRequest is not sending them. It may be due to Qt or some bug in the code. Anyway I will merge this feature with master branch now and I will see if I can fix this issue later.

Thanks for your work!

[fabiobento512]

A new version has been released with your contribution. Thanks. :)

P.S. Sorry for the huge delay but I do this for free and in my free time.