Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX late propal search option (v18+) #30687

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

W1W1-M
Copy link
Contributor

@W1W1-M W1W1-M commented Aug 20, 2024

FIX late propal search option (v18+)

Like invoices #30670, the home page link to late propal does not work correctly. Currently the link will open the propal list with all open propals, the late alert duration is not used.
This PR adds a search_option post variable for the home page link as well as a alert checkbox on the propal list.

fix-late-propal-list
fix-late-propal-list-alert

Associated issues: #25963 #26056

Merge tip: from v20+ object_statut no longer exists

@eldy eldy added the Pending analysis of PR (maintenance team) PR is in a maintenance branch with several approvers. Waiting approval of all of them. label Aug 20, 2024
@rycks
Copy link
Contributor

rycks commented Sep 5, 2024

hello @W1W1-M your PR seems really good but i think (@eldy could be more precise) there is a problem with dolibarr policy : please split your PR in two parts:

  1. one PR with part to fix the bug (modify the url filters to display -> 18.0 impact is confirmed)
  2. an other PR with alert checkbox (this is a new feature) -> that PR will be made on develop dolibarr branch

Like you did it #25963 #26056 for alert new feature

thanks,

@W1W1-M
Copy link
Contributor Author

W1W1-M commented Sep 5, 2024

hello @W1W1-M your PR seems really good but i think (@eldy could be more precise) there is a problem with dolibarr policy : please split your PR in two parts:

  1. one PR with part to fix the bug (modify the url filters to display -> 18.0 impact is confirmed)
  2. an other PR with alert checkbox (this is a new feature) -> that PR will be made on develop dolibarr branch

Like you did it #25963 #26056 for alert new feature

thanks,

Hello,
Thanks for feedback. I agree with Dolibarr policy, however the alert checkbox feels to me to be intrinsically linked to the warning on dashboard:

If a user opens the warning on the dashboard to get to the list of concerned propals, they will also need a way to un-filter the list. I suppose using the reset filters would be enough, the added logic in the list is still required. The added checkbox is only a few lines of code more. I can remove it, please confirm if : "This is the way" ⭐ ⚔️

Copy link
Contributor

@rycks rycks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok for me

@eldy eldy merged commit 4cd82df into Dolibarr:18.0 Sep 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending analysis of PR (maintenance team) PR is in a maintenance branch with several approvers. Waiting approval of all of them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants