Conversation
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
novafirst
changed the title
Contributor
|
Hey @novafirst since I released version 1.6.3 some merge conflicts would occur while dealing with your pull-request. I could merge it into 1.7, however in version 2.0 of InvoicePlane the Customer Number is offered out of the box. |
nielsdrost7
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if ( typeof options.of === "string" ) { | ||
| target = $( document ).find( options.of ); | ||
| } else { | ||
| target = $( options.of ); |
Check warning
Code scanning / CodeQL
Unsafe jQuery plugin
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application/modules/upload/controllers/Upload.php (1)
172-193: Avoid logging raw attacker‑supplied filenames.
Prefer hashing to reduce PII exposure and log‑injection risk.🔧 Suggested change
- log_message('error', 'Path traversal attempt detected in filename: ' . $filename); + $filenameHash = hash_for_logging($filename); + log_message('error', 'Path traversal attempt detected in filename (hash: ' . $filenameHash . ')');
🤖 Fix all issues with AI agents
In @.github/workflows/composer-update.yml:
- Around line 77-84: The "Check for changes" step (id: check-changes) only
inspects composer.lock via `git diff --quiet composer.lock` so it misses updates
to composer.json; update the check to detect changes to both composer.lock and
composer.json (e.g., test diffs or git status for composer.json composer.lock)
and set GITHUB_OUTPUT changes_detected accordingly so PRs are opened when either
file was modified.
In @.github/workflows/crowdin-sync.yml:
- Around line 1-105: The workflow is using the wrong branch name for Crowdin
operations: update the two occurrences of localization_branch_name currently set
to "master" to "development" inside the "Upload sources to Crowdin" and
"Download translations from Crowdin" steps (look for the
localization_branch_name key in those steps and replace its value), so Crowdin
actions target the repository's default branch.
In @.github/workflows/docker-phpmyadmin.yml:
- Around line 1-5: The workflow's triggers were changed to only
workflow_dispatch, removing automatic builds; update the top-level "on" block to
include workflow_dispatch plus automatic triggers by adding a "push" event for
the development branch and a "push" with "tags: ['v*.*.*']" (i.e., restore
push/branches and tag pushes) so the workflow runs both manually and
automatically upon branch pushes and semver tags; ensure the "on" block contains
workflow_dispatch, push.branches: [development], and push.tags: ['v*.*.*'] to
restore the original behavior.
In @.github/workflows/phpunit.yml:
- Around line 1-49: The workflow uses Laravel-specific files and commands (steps
named "Prepare Laravel environment" and "Run PHPUnit" that call php artisan and
reference .env.testing.example) which don't exist in this CodeIgniter 3.3 repo;
replace those steps with CodeIgniter-appropriate test setup: install PHPUnit via
Composer (ensure composer.json includes phpunit), add or reference a
phpunit.xml/phpunit.xml.dist suited to this project, remove php artisan
key:generate/.env.testing.example usage, and change the Run PHPUnit step to
invoke vendor/bin/phpunit (or the configured test command) and any necessary DB
setup (create sqlite or mysql service and migration/seeding commands if you add
CI test helpers). Ensure the workflow's "Setup PHP with Composer" step provides
required PHP extensions and that the final test step runs the repository's
actual test runner instead of php artisan.
In @.github/workflows/release.yml:
- Line 427: Update the GitHub Actions step that references
softprops/action-gh-release by changing the action version from v1 to v2
(replace softprops/action-gh-release@v1 with softprops/action-gh-release@v2) so
the workflow runs on Node 20; ensure any inputs or outputs used with that step
remain compatible after the version bump and adjust them if the v2 action
introduced any breaking changes.
In @.github/workflows/setup.yml:
- Around line 196-223: The workflow step with id key_generate currently runs the
Laravel command "php artisan key:generate", but this is a CodeIgniter 3 project;
replace that call with a CI-appropriate key setup: remove "php artisan
key:generate" and instead generate a random encryption key (for example via a
short PHP one-liner or openssl) and write it into your CodeIgniter config (set
$config['encryption_key'] in application/config/config.php or write to your .env
if you use one); update the step id key_generate to echo success/failure exactly
as before based on that command’s exit code. Also scan other workflow steps that
call "php artisan migrate" or "php artisan db:seed" and replace them with your
project's migration/seed approach (invoke your custom CLI controllers via "php
index.php ControllerName method" or run your project-specific migration
scripts).
In @.github/workflows/test-php.yml:
- Around line 3-15: Update the GitHub Actions workflow triggers and matrix so CI
runs on PRs and includes the project-required minimum PHP version: add
pull_request to the top-level on: triggers (in addition to workflow_dispatch)
and expand the strategy.matrix.php-versions array to include '8.1' alongside
'8.2'; confirm the matrix key names operating-system and php-versions remain
unchanged so the run job picks up the new entries.
In @.github/workflows/yarn-update.yml:
- Around line 203-211: The workflow step with id updated-packages invokes the
missing script generate-package-update-report.cjs; either add that script under
the repository (implement the logic that generates updated-packages.txt and
exits non-zero on errors) or change the step to point to the existing script
path/name that produces updated-packages.txt; update the run block to call the
correct script name and ensure the script writes updated-packages.txt before the
echo to GITHUB_OUTPUT so the "Get updated packages" step succeeds.
In `@application/helpers/file_security_helper.php`:
- Line 177: The code calls a non-existent mb_rtrim when building $fullPath in
file_security_helper.php; replace the mb_rtrim usage with PHP's rtrim() so
$fullPath = rtrim($baseDirectory, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR .
$basename['filename']; (i.e., update the expression that constructs $fullPath to
use rtrim instead of mb_rtrim).
- Line 87: The code calls a non-existent mb_rtrim which will cause a fatal
error; replace the mb_rtrim call with the standard rtrim when building
$realBaseWithSep from $realBase and DIRECTORY_SEPARATOR (e.g., update the
expression that sets $realBaseWithSep to use rtrim($realBase,
DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR) because DIRECTORY_SEPARATOR is a
single-byte ASCII char and multibyte handling is unnecessary.
In `@application/modules/clients/Enums/ClientTitleEnum.php`:
- Around line 20-31: The tryFrom method currently uses array_search on
self::getValues() and checks if ($searchResult), which skips the first enum
(index 0) and returns the index rather than the matched string; change the
lookup to perform a strict search (e.g., use array_search($value, $values, true)
or use in_array with strict true) and when a match is found return the matched
value string (the element from $values), not the numeric index; ensure tryFrom
returns null (or the expected empty/false value) when no strict match exists.
In `@application/modules/invoices/views/view.php`:
- Around line 479-498: The echoed input values for invoice_quote_number,
invoice_work_order, and invoice_agreement are not escaped and can cause XSS;
update the value outputs in the view to escape user content (e.g., wrap the
properties with htmlsc() or htmlspecialchars()) when echoing inside the value
attribute for the inputs in view.php (references: invoice_quote_number,
invoice_work_order, invoice_agreement) so the rendered value attribute uses the
escaped string.
In `@application/modules/quotes/controllers/Ajax.php`:
- Around line 21-43: In modal_quote_lookups ensure $data is always defined
before calling $this->layout->load_view to avoid the undefined variable when
$client_id is empty: either initialize $data = array() (or with default keys
'related_quotes','client_id','quote_statuses') before the if block, or add an
early return/conditional so load_view is only called when $data is set;
reference the modal_quote_lookups method, the $this->mdl_quotes->... calls, and
the final $this->layout->load_view('quotes/modal_quote_lookups', $data) when
applying the fix.
- Around line 144-146: The POST-extracted variables $quote_work_order and
$quote_agreement are never added to the record being saved; update the
controller method in application/modules/quotes/controllers/Ajax.php to include
these keys in the $db_array (e.g., 'quote_work_order' => $quote_work_order and
'quote_agreement' => $quote_agreement) before the save/insert call so the values
persist to the DB; ensure the $db_array used by the existing save/update
function contains those two fields and run existing migrations/column names to
match.
In `@application/modules/quotes/views/modal_quote_lookups.php`:
- Around line 9-29: Validate that a quote is selected in the
.select-items-confirm click handler by checking quote_id (from
$("input[name='quote_id']:checked").val()) and aborting with a user-facing
message if invalid; wrap the $.post callback parsing in a try/catch around
JSON.parse(data) and handle parse or AJAX errors by logging and showing an error
to the user; replace the for..in loop over items with a deterministic iteration
(e.g., use items[0] or a for loop over numeric indices) so all fields are set
correctly and move $('#modal-choose-quote').modal('hide') outside the loop so
the modal is closed only after processing completes; ensure the error-handling
branches do not attempt to use the parsed items when parsing fails.
In `@application/modules/quotes/views/view.php`:
- Around line 349-362: The quote_work_order and quote_agreement values are
echoed into input value attributes without escaping, enabling XSS; update the
view (inputs with id "quote_work_order" and "quote_agreement" in view.php) to
output escaped values using a safe HTML-escaping function (e.g.
htmlspecialchars($quote->quote_work_order, ENT_QUOTES, 'UTF-8') and
htmlspecialchars($quote->quote_agreement, ENT_QUOTES, 'UTF-8') or your
framework's html_escape helper) and always render the value attribute (even when
empty) to avoid malformed HTML.
In `@application/modules/setup/sql/041_1.7.0.sql`:
- Line 1: The migration file 041_1.7.0.sql currently contains only a comment;
add ALTER TABLE statements to create the missing columns used by the app: add
client_number (e.g., VARCHAR(255) DEFAULT NULL) to ip_clients, add
invoice_quote_number (TEXT DEFAULT NULL), invoice_work_order (VARCHAR(255)
DEFAULT NULL) and invoice_agreement (VARCHAR(255) DEFAULT NULL) to ip_invoices,
and add quote_work_order (VARCHAR(255) DEFAULT NULL) and quote_agreement
(VARCHAR(255) DEFAULT NULL) to ip_quotes; ensure the statements are idempotent
for your DB flavor (or wrapped in a safe conditional/transaction) and follow the
repository’s naming and datatype conventions so functions referencing these
fields (client_number, invoice_quote_number, invoice_work_order,
invoice_agreement, quote_work_order, quote_agreement) will find the columns at
runtime.
In `@application/views/quote_templates/public/InvoicePlane_Web.php`:
- Around line 79-83: Escape the user-supplied VAT/tax ID and attachment name
outputs to prevent stored XSS by replacing direct echoes of $quote->user_vat_id
and $quote->user_tax_code (and any attachment name echoes around the other
mentioned lines) with an HTML-escaped variant; use the project's escaping helper
(e.g., html_escape()) or PHP's htmlspecialchars(..., ENT_QUOTES, 'UTF-8') when
outputting these variables so all printed values are properly encoded before
echoing.
In `@assets/core/js/jquery-ui.js`:
- Around line 888-894: The .position() widget is vulnerable to XSS when a string
passed to options.of is handed directly to $() — change handling so that when
options.of is a string you call $(document).find(options.of) (ensuring strings
are treated only as selectors) and otherwise continue to pass non-string values
(DOM element, jQuery object, Event, window/document) directly to $(); update the
branch around options.of and target to reflect this secure pattern and consider
replacing the vendored assets/core/js/jquery-ui.js with jQuery UI 1.13.0+ which
includes this fix upstream.
In `@assets/core/js/scripts.js`:
- Around line 76-77: The preview currently uses .text() in
update_email_template_preview(), which escapes HTML and shows raw tags; change
it to write sanitized HTML into the iframe/body (use .html() on
$('#email-template-preview').contents().find('body')) after running the template
content through your sanitization function (e.g., sanitizeHtml or a safeSanitize
helper) to prevent XSS while allowing tags to render; update references to
$('.email-template-body').val() as the input to the sanitizer before injecting.
♻️ Duplicate comments (2)
application/modules/invoices/views/view.php (1)
468-499: Accessibility: Input fields are missing accessible labels.The
<label>elements are not properly associated with their corresponding<input>elements viaforattributes. This makes the form less accessible for screen readers and voice control users.Suggested fix
<div class="col-xs-12 col-md-4"> <div class="invoice-properties"> - <label><?php _trans('related_quote_number'); ?></label> + <label for="invoice_quote_number"><?php _trans('related_quote_number'); ?></label> ... </div> <div class="invoice-properties"> - <label><?php _trans('work_order'); ?></label> + <label for="invoice_work_order"><?php _trans('work_order'); ?></label> ... </div> <div class="invoice-properties"> - <label><?php _trans('agreement'); ?></label> + <label for="invoice_agreement"><?php _trans('agreement'); ?></label> ... </div> </div>application/modules/quotes/views/view.php (1)
349-350: Accessibility: Input fields missing associated labelforattribute.The labels for
quote_work_orderandquote_agreementinputs don't have aforattribute linking them to their respective inputs, which impacts screen reader users.
🟡 Minor comments (9)
application/helpers/client_helper.php-45-56 (1)
45-56: Add null safety and use strict comparison.The function lacks null safety checks that
format_client()above implements. If$clientis null or missing theclient_numberproperty, this will throw an error.Additionally, consider using strict comparison (
!== "") orempty()for consistency with PHP best practices.Proposed fix with null safety
/** * `@param` object $client * `@return` string */ -function format_client_number($client) +function format_client_number($client): string { - if ($client->client_number != "") { + if (!empty($client->client_number)) { return $client->client_number; } return "00-00000"; }.github/workflows/README.md-221-226 (1)
221-226: Minor inconsistency in PHP version documentation.The documentation mentions "PHP 8.2" but the actual workflow (
pint.yml) uses PHP 8.4. Consider updating for consistency.**What it does:** 1. **Checks out the PR branch** - Gets the latest code from the pull request -2. **Sets up PHP environment** - Installs PHP 8.2 with required extensions +2. **Sets up PHP environment** - Installs PHP 8.4 with required extensions.github/workflows/phpstan.yml-65-93 (1)
65-93: Dead code: PR comment step will never execute.Since this workflow only triggers on
workflow_dispatch, the conditiongithub.event_name == 'pull_request'will never be true. Either addpull_requestto the triggers or remove this step.If you intend to support PR comments in the future, add the trigger:
on: workflow_dispatch: # Manual only — does not run automatically + pull_request: + branches: + - master + - developOtherwise, remove the dead code step (lines 65-93).
.github/workflows/setup.yml-371-374 (1)
371-374: Unreachable code:echo "::endgroup::"will never execute.The
exit 0andexit 1statements on lines 350 and 371 terminate the script before the::endgroup::echo can be reached.Suggested fix - move endgroup before exit statements
exit 0 fi - - echo "::endgroup::"The
::endgroup::at line 374 can be safely removed since the::group::at line 287 will be implicitly closed when the step ends.application/views/quote_templates/public/InvoicePlane_Web.php-215-216 (1)
215-216: Use$colspanfor discount rows to keep column alignment.The hardcoded colspan causes misalignment when item discounts are hidden.
🧩 Suggested fix
- <td class="no-bottom-border" colspan="4"></td> + <td class="no-bottom-border" colspan="<?php echo $colspan; ?>"></td> ... - <td class="no-bottom-border" colspan="4"></td> + <td class="no-bottom-border" colspan="<?php echo $colspan; ?>"></td>Also applies to: 267-268
.github/workflows/composer-update.yml-50-58 (1)
50-58: Composer audit output check always flags vulnerabilities.The
[ -s audit-report.json ]check is unreliable. Even when no vulnerabilities are found,composer audit --format=jsonoutputs valid JSON with an empty advisories object (e.g.,{"advisories": {}}), which is still a non-empty file. This causes the flag to always be set to true.Parse the JSON instead to check if the
advisoriesobject actually contains findings.Suggested fix
- if [ -s audit-report.json ]; then - echo "vulnerabilities_found=true" >> $GITHUB_OUTPUT - else - echo "vulnerabilities_found=false" >> $GITHUB_OUTPUT - fi + php -r '$data=json_decode(file_get_contents("audit-report.json"), true) ?: []; $has = !empty($data["advisories"]); echo "vulnerabilities_found=" . ($has ? "true" : "false") . PHP_EOL;' >> $GITHUB_OUTPUTapplication/modules/quotes/controllers/Ajax.php-52-53 (1)
52-53: Ineffectivestatuses()call inmodal_quote_lookups_select.Same issue as above - the
statuses()return value is discarded.🐛 Remove the ineffective call
if (!empty($quote_id)) { $this->mdl_quotes->is_related_quote($quote_id); - $this->mdl_quotes->statuses(); $quote = $this->mdl_quotes->get()->result(); echo json_encode($quote); }application/modules/quotes/views/modal_quote_lookups.php-42-42 (1)
42-42: Fix ARIA attributes for accessibility.The
aria-labelledbyattribute should reference theidof the element containing the modal's title, not the modal's own id. Also, the modal should havetabindex="-1"for proper focus management.♿ Proposed fix
-<div id="modal-choose-quote" class="modal col-xs-12 col-sm-10 col-sm-offset-1" role="dialog" aria-labelledby="modal-choose-quote" aria-hidden="true"> +<div id="modal-choose-quote" class="modal col-xs-12 col-sm-10 col-sm-offset-1" role="dialog" aria-labelledby="modal-choose-quote-title" aria-hidden="true" tabindex="-1"> <form class="modal-content"> <div class="modal-header"> <button type="button" class="close" data-dismiss="modal"><i class="fa fa-close"></i></button> - <h4 class="panel-title"><?php _trans('select_related_quote'); ?></h4> + <h4 class="panel-title" id="modal-choose-quote-title"><?php _trans('select_related_quote'); ?></h4> </div>application/modules/quotes/controllers/Ajax.php-29-30 (1)
29-30: Ineffectivestatuses()call - return value not used.The
statuses()method returns an array but the return value is not assigned to anything. This call has no effect.🐛 Remove the ineffective call
$this->mdl_quotes->by_client($client_id); $this->mdl_quotes->related_is_open(); - $this->mdl_quotes->statuses(); $quotes = $this->mdl_quotes->get()->result();
🧹 Nitpick comments (11)
application/third_party/MX/Controller.php (1)
55-55: Simplify the no-op string replacement.The
str_replace('', '', get_class($this))call replaces an empty string with an empty string, which is effectively a no-op. This makes the code less readable.♻️ Simplify to direct assignment
- $class = str_replace('', '', get_class($this)); + $class = get_class($this);.github/workflows/docker.yml (1)
23-27: Preferdocker composeor ensuredocker-composeis installed.Ubuntu runners have moved to Compose v2; the
docker-composebinary may be missing. Consider switching todocker composeor adding an install step.♻️ Proposed update
- - name: Build and start containers (if docker-compose.yml is ready) - run: docker-compose up -d --build + - name: Build and start containers (if docker-compose.yml is ready) + run: docker compose up -d --build - - name: Check running services - run: docker-compose ps + - name: Check running services + run: docker compose pscomposer.json (1)
20-20: Consider using a caret constraint forpocketarc/codeigniter.The fixed version
"3.3"won't receive patch updates automatically, unlike other dependencies using caret constraints (e.g.,^2.6). Consider using"^3.3"to allow patch-level updates while maintaining compatibility.Suggested change
- "pocketarc/codeigniter": "3.3", + "pocketarc/codeigniter": "^3.3",.github/workflows/pint.yml (2)
70-76: Parse error detection may have false positives.Grepping for
"!"in the output is too broad and may match legitimate content (e.g., file paths, output messages). Consider using a more specific pattern or relying solely on the exit code and "Parse error" string.Suggested improvement
# Check if there were parse errors - if grep -q "Parse error" pint_output.log || grep -q "!" pint_output.log; then + if grep -qE "Parse error|^\s+!" pint_output.log; then echo "parse_errors=true" >> $GITHUB_OUTPUT echo "WARNING: Parse errors detected in one or more files" elseOr alternatively, remove the
"!"check entirely and rely on the more specific "Parse error" pattern.
93-107: Consider handling push failures gracefully.If the
git pushfails (e.g., due to branch protection rules or concurrent updates), the workflow will fail without clear guidance. Consider adding error handling.Suggested improvement
git config --local user.email "github-actions[bot]@users.noreply.github.com" git config --local user.name "github-actions[bot]" git add -A git commit -m "style: apply Laravel Pint fixes" --no-verify - git push + git push || { + echo "::warning::Failed to push changes. This may be due to branch protection rules or concurrent updates." + echo "Please pull the latest changes and run Pint locally." + exit 1 + }.github/workflows/setup.yml (1)
102-108: Minor: Missing newline before Step 1 comment.There's a missing blank line between the initialization step output and the Step 1 comment, which slightly reduces readability.
echo "errors_found=false" >> $GITHUB_OUTPUT echo "::notice::Starting setup with error tracking enabled" + # Step 1: Yarn Install.github/workflows/phpstan.yml (1)
50-54: Fail step could be more informative.Consider outputting a summary of the error count or directing users to the artifacts for details.
Suggested improvement
- name: Check for errors and fail if found if: steps.phpstan-analysis.outcome == 'failure' run: | echo "PHPStan found errors in the codebase" + echo "See the uploaded artifacts for detailed results" + echo "Download phpstan-report.md for a formatted summary" exit 1.github/workflows/yarn-update.yml (2)
219-220: Consider enabling automatic branch deletion.Setting
delete-branch: falsewill leave automated branches (e.g.,automated/yarn-update-*) in the repository after the PR is merged. Over time, this can lead to branch clutter.💡 Suggested change
branch: automated/yarn-update-${{ github.run_number }} - delete-branch: false + delete-branch: true
107-136: Security update logic relies on parsing audit output format.The security update step parses
audit-report.jsonusinggrepto extract vulnerable package names. This approach is brittle and may break if Yarn's audit JSON format changes. Additionally, upgrading vulnerable packages to--latestmay introduce breaking changes.Consider pinning to compatible versions rather than always using
--latest:for pkg in $VULNERABLE_PACKAGES; do echo " Upgrading $pkg..." - yarn upgrade "$pkg" --latest || true + yarn upgrade "$pkg" || true doneapplication/modules/quotes/models/Mdl_quotes.php (1)
345-352: Consider adding type hint for$quote_idparameter.The method follows existing patterns in this file, but adding a type hint would improve type safety and documentation.
💡 Suggested improvement
/** + * `@param` int $quote_id * `@return` $this */ - public function is_related_quote($quote_id) + public function is_related_quote(int $quote_id) { $this->filter_where('ip_quotes.quote_id', $quote_id); return $this; }application/modules/quotes/views/modal_quote_lookups.php (1)
13-14: Remove commented-out debug code.These commented lines appear to be leftover from development and should be removed.
🧹 Suggested cleanup
- //$.post("<?php echo site_url('quotes/ajax/process_product_selections'); ?>", { - //$.post("<?php echo site_url('quotes/ajax/modal_quote_lookups_select'); ?>", { - $.post("<?php echo site_url('quotes/ajax/modal_quote_lookups_select'); ?>", {
Comment on lines
+77
to
+84
| - name: Check for changes | ||
| id: check-changes | ||
| run: | | ||
| if git diff --quiet composer.lock; then | ||
| echo "changes_detected=false" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "changes_detected=true" >> $GITHUB_OUTPUT | ||
| fi |
Contributor
There was a problem hiding this comment.
Detect composer.json changes when deciding to open a PR.
composer bump can update composer.json without a lockfile change; the current check would skip PR creation.
🛠️ Suggested fix
- if git diff --quiet composer.lock; then
+ if git diff --quiet composer.lock composer.json; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Check for changes | |
| id: check-changes | |
| run: | | |
| if git diff --quiet composer.lock; then | |
| echo "changes_detected=false" >> $GITHUB_OUTPUT | |
| else | |
| echo "changes_detected=true" >> $GITHUB_OUTPUT | |
| fi | |
| - name: Check for changes | |
| id: check-changes | |
| run: | | |
| if git diff --quiet composer.lock composer.json; then | |
| echo "changes_detected=false" >> $GITHUB_OUTPUT | |
| else | |
| echo "changes_detected=true" >> $GITHUB_OUTPUT | |
| fi |
🤖 Prompt for AI Agents
In @.github/workflows/composer-update.yml around lines 77 - 84, The "Check for
changes" step (id: check-changes) only inspects composer.lock via `git diff
--quiet composer.lock` so it misses updates to composer.json; update the check
to detect changes to both composer.lock and composer.json (e.g., test diffs or
git status for composer.json composer.lock) and set GITHUB_OUTPUT
changes_detected accordingly so PRs are opened when either file was modified.
Comment on lines
+1
to
+105
| name: Crowdin Translation Sync | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| action: | ||
| description: 'Action to perform' | ||
| required: true | ||
| type: choice | ||
| options: | ||
| - upload-sources | ||
| - download-translations | ||
| - sync-bidirectional | ||
| default: 'download-translations' | ||
| schedule: | ||
| # Run weekly on Sundays at 2:00 AM UTC | ||
| - cron: '0 2 * * 0' | ||
|
|
||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
|
|
||
| # Note: This workflow requires a Personal Access Token (PAT) to create pull requests. | ||
| # The default GITHUB_TOKEN has restricted permissions and cannot create PRs that trigger other workflows. | ||
| # | ||
| # To configure the required secret: | ||
| # 1. Create a Personal Access Token (classic) with 'repo' and 'workflow' scopes | ||
| # at https://github.com/settings/tokens | ||
| # 2. Add the token as a repository secret named 'PAT_TOKEN' | ||
| # at https://github.com/OWNER/REPO/settings/secrets/actions | ||
| # | ||
| # See: https://github.com/crowdin/github-action | ||
|
|
||
| jobs: | ||
| crowdin-sync: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Upload sources to Crowdin | ||
| if: github.event.inputs.action == 'upload-sources' || github.event.inputs.action == 'sync-bidirectional' | ||
| uses: crowdin/github-action@v2 | ||
| with: | ||
| upload_sources: true | ||
| upload_translations: false | ||
| download_translations: false | ||
| localization_branch_name: master | ||
| config: 'crowdin.yml' | ||
| project_id: ${{ secrets.CROWDIN_PROJECT_ID }} | ||
| token: ${{ secrets.CROWDIN_PERSONAL_TOKEN }} | ||
|
|
||
| - name: Download translations from Crowdin | ||
| if: github.event.inputs.action == 'download-translations' || github.event.inputs.action == 'sync-bidirectional' || github.event_name == 'schedule' | ||
| uses: crowdin/github-action@v2 | ||
| with: | ||
| upload_sources: false | ||
| upload_translations: false | ||
| download_translations: true | ||
| localization_branch_name: master | ||
| create_pull_request: true | ||
| pull_request_title: 'chore(i18n): update translations from Crowdin' | ||
| pull_request_body: | | ||
| ## Translation Update | ||
|
|
||
| This PR updates translations downloaded from Crowdin. | ||
|
|
||
| **Triggered by:** ${{ github.event_name }} | ||
| **Action:** ${{ github.event.inputs.action || 'download-translations' }} | ||
|
|
||
| ### Review Checklist | ||
|
|
||
| - [ ] Review translation changes for accuracy | ||
| - [ ] Check for any formatting issues | ||
| - [ ] Verify no code is affected | ||
| - [ ] Test translations in UI if possible | ||
|
|
||
| --- | ||
|
|
||
| *This PR was automatically created by the Crowdin Sync workflow.* | ||
| pull_request_labels: | | ||
| i18n | ||
| translations | ||
| crowdin | ||
| automated-pr | ||
| config: 'crowdin.yml' | ||
| project_id: ${{ secrets.CROWDIN_PROJECT_ID }} | ||
| token: ${{ secrets.CROWDIN_PERSONAL_TOKEN }} | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.PAT_TOKEN }} | ||
|
|
||
| - name: Workflow summary | ||
| run: | | ||
| echo "## Crowdin Sync Complete" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Action:** ${{ github.event.inputs.action || 'download-translations' }}" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Triggered by:** ${{ github.event_name }}" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "### Next Steps" >> $GITHUB_STEP_SUMMARY | ||
| echo "- Review the created pull request (if any)" >> $GITHUB_STEP_SUMMARY | ||
| echo "- Verify translation changes" >> $GITHUB_STEP_SUMMARY | ||
| echo "- Merge when ready" >> $GITHUB_STEP_SUMMARY |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check available branches
echo "=== Available Branches ==="
git branch -a 2>/dev/null || echo "No git branches available"
# Check localization_branch_name references in the workflow
echo -e "\n=== localization_branch_name in workflow ==="
rg -n 'localization_branch_name' .github/workflows/crowdin-sync.yml
# Check for crowdin.yml or crowdin.yaml config file
echo -e "\n=== Crowdin config files ==="
fd -a '(crowdin\.ya?ml|crowdin\.json)$' . -x cat {}
# Check default branch in repo config
echo -e "\n=== Repository Default Branch ==="
cat .git/config 2>/dev/null | grep -A5 'branch' || echo "Git config not available"
# Check common branch name patterns in repository
echo -e "\n=== Branch name patterns in README/docs ==="
fd -a 'README.md' . -x rg -i 'main branch|default branch|development|main|master' {} || echo "No README matches"Repository: InvoicePlane/InvoicePlane
Length of output: 1265
Change localization_branch_name from master to development.
The repository's default branch is development, not master. The hardcoded localization_branch_name: master on lines 51 and 63 references a non-existent branch, which will cause the workflow to fail when attempting to create pull requests. Update both instances to localization_branch_name: development.
🤖 Prompt for AI Agents
In @.github/workflows/crowdin-sync.yml around lines 1 - 105, The workflow is
using the wrong branch name for Crowdin operations: update the two occurrences
of localization_branch_name currently set to "master" to "development" inside
the "Upload sources to Crowdin" and "Download translations from Crowdin" steps
(look for the localization_branch_name key in those steps and replace its
value), so Crowdin actions target the repository's default branch.
Comment on lines
+888
to
+894
| // Ensure that string values for options.of are treated strictly as selectors | ||
| // and are not interpreted as HTML by the jQuery constructor. | ||
| if ( typeof options.of === "string" ) { | ||
| target = $( document ).find( options.of ); | ||
| } else { | ||
| target = $( options.of ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CVE-2022-31160 jQuery UI XSS vulnerability
💡 Result:
Short summary — CVE-2022-31160 (jQuery UI)
-
What it is: a reflected/stored XSS in jQuery UI's checkboxradio widget. When a checkboxradio is initialized on an input inside a label, calling .checkboxradio("refresh") can incorrectly decode encoded HTML entities in the label text and thereby execute JavaScript. [1][2]
-
Affected versions: jquery-ui < 1.13.2. Patched in jQuery UI 1.13.2. [1][2]
-
Severity / impact: CVSSv3 ~6.1 (Medium); network-exploitable but requires user interaction in many scenarios. [1][3]
-
Mitigation / fix: upgrade to jQuery UI 1.13.2 (or later). As a workaround, wrap non-input contents of the label in a to prevent the unsafe decoding. [1][2][4]
References
- NVD / CVE-2022-31160.
- GitHub Advisory (jquery-ui) — GHSA-h6gj-6jjq-h8g9.
- Debian / Ubuntu security tracker entries.
- Vendor/third‑party advisories describing the mitigation and patch.
🌐 Web query:
jQuery UI 1.13.0 security patches CVE position widget
💡 Result:
Short answer — yes: jQuery UI 1.13.0 fixed multiple XSS issues (including the .position() "of" option). Specifically:
- CVE-2021-41184 — XSS in the "of" option of the .position() util; fixed in 1.13.0. [1][2]
- CVE-2021-41182 — XSS in Datepicker altField; fixed in 1.13.0. [1][3]
- CVE-2021-41183 — XSS in Datepicker *Text options; fixed in 1.13.0. [1][3]
Note: a later XSS (CVE-2022-31160) affecting checkboxradio.refresh was fixed in jQuery UI 1.13.2. [4]
Sources:
[1] jquery/jquery-ui security advisories (GitHub).
[2] CVE-2021-41184 advisories / OSV / vendor pages.
[3] CVE-2021-41182 / CVE-2021-41183 entries (NVD/OSV).
[4] CVE-2022-31160 (package advisories / Rapid7 / OSV).
🌐 Web query:
jQuery UI 1.12.1 to 1.13.0 upgrade security fixes
💡 Result:
Summary — what changed (security fixes)
- jQuery UI 1.13.0 fixed three XSS vulnerabilities present in 1.12.1:
- CVE-2021-41182 — XSS via Datepicker altField. [1][3]
- CVE-2021-41183 — XSS via various Datepicker "*Text" options (closeText, currentText, prev/nextText, buttonText, appendText). [1][3]
- CVE-2021-41184 — XSS via the of option of the .position() utility. [1][6]
Recommendation
- Upgrade any jQuery UI < 1.13.0 to 1.13.0 or later (patches 1.13.1/1.13.2 followed and included further security/regression fixes). [2][5][4]
Sources
- jQuery UI 1.13.0 release notes / upgrade guide. [1][2]
- Snyk vulnerability page for jquery-ui 1.12.1. [3]
- Tenable advisory summarizing the three CVEs. [4]
- jQuery UI blog posts for 1.13.x patch releases. [5]
Security fix for XSS vulnerability in .position() widget — verify alignment with official patches.
This change correctly mitigates CVE-2021-41184, a known jQuery UI XSS vulnerability where passing a string directly to $() in the .position() widget's of option could interpret it as HTML instead of a selector. Using $(document).find(options.of) ensures string inputs are treated strictly as selectors.
Regarding the CodeQL warning on line 893: this appears to be a false positive. The else branch only executes when options.of is not a string (e.g., DOM element, jQuery object, Event object, or window/document), all of which are safe to pass to $().
Consider upgrading to jQuery UI 1.13.0+ instead of maintaining manual patches to this vendored file, as 1.13.0 includes this fix natively.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 893-893: Unsafe jQuery plugin
Potential XSS vulnerability in the '$.fn.position' plugin.
🤖 Prompt for AI Agents
In `@assets/core/js/jquery-ui.js` around lines 888 - 894, The .position() widget
is vulnerable to XSS when a string passed to options.of is handed directly to
$() — change handling so that when options.of is a string you call
$(document).find(options.of) (ensuring strings are treated only as selectors)
and otherwise continue to pass non-string values (DOM element, jQuery object,
Event, window/document) directly to $(); update the branch around options.of and
target to reflect this secure pattern and consider replacing the vendored
assets/core/js/jquery-ui.js with jQuery UI 1.13.0+ which includes this fix
upstream.
nielsdrost7
requested changes
Jan 19, 2026
Contributor
nielsdrost7
left a comment
nielsdrost7
left a comment
There was a problem hiding this comment.
In @.github/workflows/composer-update.yml:
- Around line 77-84: The "Check for changes" step (id: check-changes) only
inspects composer.lock viagit diff --quiet composer.lockso it misses updates
to composer.json; update the check to detect changes to both composer.lock and
composer.json (e.g., test diffs or git status for composer.json composer.lock)
and set GITHUB_OUTPUT changes_detected accordingly so PRs are opened when either
file was modified.
In @.github/workflows/crowdin-sync.yml:
- Around line 1-105: The workflow is using the wrong branch name for Crowdin
operations: update the two occurrences of localization_branch_name currently set
to "master" to "development" inside the "Upload sources to Crowdin" and
"Download translations from Crowdin" steps (look for the
localization_branch_name key in those steps and replace its value), so Crowdin
actions target the repository's default branch.
In @.github/workflows/docker-phpmyadmin.yml:
- Around line 1-5: The workflow's triggers were changed to only
workflow_dispatch, removing automatic builds; update the top-level "on" block to
include workflow_dispatch plus automatic triggers by adding a "push" event for
the development branch and a "push" with "tags: ['v*..']" (i.e., restore
push/branches and tag pushes) so the workflow runs both manually and
automatically upon branch pushes and semver tags; ensure the "on" block contains
workflow_dispatch, push.branches: [development], and push.tags: ['v*..'] to
restore the original behavior.
In @.github/workflows/phpunit.yml:
- Around line 1-49: The workflow uses Laravel-specific files and commands (steps
named "Prepare Laravel environment" and "Run PHPUnit" that call php artisan and
reference .env.testing.example) which don't exist in this CodeIgniter 3.3 repo;
replace those steps with CodeIgniter-appropriate test setup: install PHPUnit via
Composer (ensure composer.json includes phpunit), add or reference a
phpunit.xml/phpunit.xml.dist suited to this project, remove php artisan
key:generate/.env.testing.example usage, and change the Run PHPUnit step to
invoke vendor/bin/phpunit (or the configured test command) and any necessary DB
setup (create sqlite or mysql service and migration/seeding commands if you add
CI test helpers). Ensure the workflow's "Setup PHP with Composer" step provides
required PHP extensions and that the final test step runs the repository's
actual test runner instead of php artisan.
In @.github/workflows/release.yml:
- Line 427: Update the GitHub Actions step that references
softprops/action-gh-release by changing the action version from v1 to v2
(replace softprops/action-gh-release@v1 with softprops/action-gh-release@v2) so
the workflow runs on Node 20; ensure any inputs or outputs used with that step
remain compatible after the version bump and adjust them if the v2 action
introduced any breaking changes.
In @.github/workflows/setup.yml:
- Around line 196-223: The workflow step with id key_generate currently runs the
Laravel command "php artisan key:generate", but this is a CodeIgniter 3 project;
replace that call with a CI-appropriate key setup: remove "php artisan
key:generate" and instead generate a random encryption key (for example via a
short PHP one-liner or openssl) and write it into your CodeIgniter config (set
$config['encryption_key'] in application/config/config.php or write to your .env
if you use one); update the step id key_generate to echo success/failure exactly
as before based on that command’s exit code. Also scan other workflow steps that
call "php artisan migrate" or "php artisan db:seed" and replace them with your
project's migration/seed approach (invoke your custom CLI controllers via "php
index.php ControllerName method" or run your project-specific migration
scripts).
In @.github/workflows/test-php.yml:
- Around line 3-15: Update the GitHub Actions workflow triggers and matrix so CI
runs on PRs and includes the project-required minimum PHP version: add
pull_request to the top-level on: triggers (in addition to workflow_dispatch)
and expand the strategy.matrix.php-versions array to include '8.1' alongside
'8.2'; confirm the matrix key names operating-system and php-versions remain
unchanged so the run job picks up the new entries.
In @.github/workflows/yarn-update.yml:
- Around line 203-211: The workflow step with id updated-packages invokes the
missing script generate-package-update-report.cjs; either add that script under
the repository (implement the logic that generates updated-packages.txt and
exits non-zero on errors) or change the step to point to the existing script
path/name that produces updated-packages.txt; update the run block to call the
correct script name and ensure the script writes updated-packages.txt before the
echo to GITHUB_OUTPUT so the "Get updated packages" step succeeds.
In @application/helpers/file_security_helper.php:
- Line 177: The code calls a non-existent mb_rtrim when building $fullPath in
file_security_helper.php; replace the mb_rtrim usage with PHP's rtrim() so
$fullPath = rtrim($baseDirectory, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR .
$basename['filename']; (i.e., update the expression that constructs $fullPath to
use rtrim instead of mb_rtrim). - Line 87: The code calls a non-existent mb_rtrim which will cause a fatal
error; replace the mb_rtrim call with the standard rtrim when building
$realBaseWithSep from $realBase and DIRECTORY_SEPARATOR (e.g., update the
expression that sets $realBaseWithSep to use rtrim($realBase,
DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR) because DIRECTORY_SEPARATOR is a
single-byte ASCII char and multibyte handling is unnecessary.
In @application/modules/clients/Enums/ClientTitleEnum.php:
- Around line 20-31: The tryFrom method currently uses array_search on
self::getValues() and checks if ($searchResult), which skips the first enum
(index 0) and returns the index rather than the matched string; change the
lookup to perform a strict search (e.g., use array_search($value, $values, true)
or use in_array with strict true) and when a match is found return the matched
value string (the element from $values), not the numeric index; ensure tryFrom
returns null (or the expected empty/false value) when no strict match exists.
In @application/modules/invoices/views/view.php:
- Around line 479-498: The echoed input values for invoice_quote_number,
invoice_work_order, and invoice_agreement are not escaped and can cause XSS;
update the value outputs in the view to escape user content (e.g., wrap the
properties with htmlsc() or htmlspecialchars()) when echoing inside the value
attribute for the inputs in view.php (references: invoice_quote_number,
invoice_work_order, invoice_agreement) so the rendered value attribute uses the
escaped string.
In @application/modules/quotes/controllers/Ajax.php:
- Around line 21-43: In modal_quote_lookups ensure $data is always defined
before calling $this->layout->load_view to avoid the undefined variable when
$client_id is empty: either initialize $data = array() (or with default keys
'related_quotes','client_id','quote_statuses') before the if block, or add an
early return/conditional so load_view is only called when $data is set;
reference the modal_quote_lookups method, the $this->mdl_quotes->... calls, and
the final $this->layout->load_view('quotes/modal_quote_lookups', $data) when
applying the fix. - Around line 144-146: The POST-extracted variables $quote_work_order and
$quote_agreement are never added to the record being saved; update the
controller method in application/modules/quotes/controllers/Ajax.php to include
these keys in the $db_array (e.g., 'quote_work_order' => $quote_work_order and
'quote_agreement' => $quote_agreement) before the save/insert call so the values
persist to the DB; ensure the $db_array used by the existing save/update
function contains those two fields and run existing migrations/column names to
match.
In @application/modules/quotes/views/modal_quote_lookups.php:
- Around line 9-29: Validate that a quote is selected in the
.select-items-confirm click handler by checking quote_id (from
$("input[name='quote_id']:checked").val()) and aborting with a user-facing
message if invalid; wrap the $.post callback parsing in a try/catch around
JSON.parse(data) and handle parse or AJAX errors by logging and showing an error
to the user; replace the for..in loop over items with a deterministic iteration
(e.g., use items[0] or a for loop over numeric indices) so all fields are set
correctly and move $('#modal-choose-quote').modal('hide') outside the loop so
the modal is closed only after processing completes; ensure the error-handling
branches do not attempt to use the parsed items when parsing fails.
In @application/modules/quotes/views/view.php:
- Around line 349-362: The quote_work_order and quote_agreement values are
echoed into input value attributes without escaping, enabling XSS; update the
view (inputs with id "quote_work_order" and "quote_agreement" in view.php) to
output escaped values using a safe HTML-escaping function (e.g.
htmlspecialchars($quote->quote_work_order, ENT_QUOTES, 'UTF-8') and
htmlspecialchars($quote->quote_agreement, ENT_QUOTES, 'UTF-8') or your
framework's html_escape helper) and always render the value attribute (even when
empty) to avoid malformed HTML.
In @application/modules/setup/sql/041_1.7.0.sql:
- Line 1: The migration file 041_1.7.0.sql currently contains only a comment;
add ALTER TABLE statements to create the missing columns used by the app: add
client_number (e.g., VARCHAR(255) DEFAULT NULL) to ip_clients, add
invoice_quote_number (TEXT DEFAULT NULL), invoice_work_order (VARCHAR(255)
DEFAULT NULL) and invoice_agreement (VARCHAR(255) DEFAULT NULL) to ip_invoices,
and add quote_work_order (VARCHAR(255) DEFAULT NULL) and quote_agreement
(VARCHAR(255) DEFAULT NULL) to ip_quotes; ensure the statements are idempotent
for your DB flavor (or wrapped in a safe conditional/transaction) and follow the
repository’s naming and datatype conventions so functions referencing these
fields (client_number, invoice_quote_number, invoice_work_order,
invoice_agreement, quote_work_order, quote_agreement) will find the columns at
runtime.
In @application/views/quote_templates/public/InvoicePlane_Web.php:
- Around line 79-83: Escape the user-supplied VAT/tax ID and attachment name
outputs to prevent stored XSS by replacing direct echoes of $quote->user_vat_id
and $quote->user_tax_code (and any attachment name echoes around the other
mentioned lines) with an HTML-escaped variant; use the project's escaping helper
(e.g., html_escape()) or PHP's htmlspecialchars(..., ENT_QUOTES, 'UTF-8') when
outputting these variables so all printed values are properly encoded before
echoing.
In @assets/core/js/jquery-ui.js:
- Around line 888-894: The .position() widget is vulnerable to XSS when a string
passed to options.of is handed directly to $() — change handling so that when
options.of is a string you call $(document).find(options.of) (ensuring strings
are treated only as selectors) and otherwise continue to pass non-string values
(DOM element, jQuery object, Event, window/document) directly to $(); update the
branch around options.of and target to reflect this secure pattern and consider
replacing the vendored assets/core/js/jquery-ui.js with jQuery UI 1.13.0+ which
includes this fix upstream.
In @assets/core/js/scripts.js:
- Around line 76-77: The preview currently uses .text() in
update_email_template_preview(), which escapes HTML and shows raw tags; change
it to write sanitized HTML into the iframe/body (use .html() on
$('#email-template-preview').contents().find('body')) after running the template
content through your sanitization function (e.g., sanitizeHtml or a safeSanitize
helper) to prevent XSS while allowing tags to render; update references to
$('.email-template-body').val() as the input to the sanitizer before injecting.
Contributor
|
@nielsdrost7 I've opened a new pull request, #1394, to work on those changes. Once the pull request is ready, I'll request review from you. |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds comprehensive GitHub Actions CI/CD infrastructure, implements quote-related features (work order and agreement fields), migrates from codeigniter/framework to pocketarc/codeigniter, and applies widespread code formatting improvements. It includes new database migrations, a quote lookup modal UI, security fixes for HTML sanitization, and configuration updates across build and deployment workflows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application/modules/sessions/controllers/Sessions.php (1)
451-556: Add explicit PHP version constraint to composer.json.The codebase uses
str_contains()andstr_starts_with(), which require PHP 8.0+. While the Docker configuration specifies PHP 8.1,composer.jsonlacks an explicit PHP version requirement in the "require" section. This allows installations without the required runtime, causing fatal errors. Add"php": ">=8.1"to composer.json to enforce the minimum version requirement.
🤖 Fix all issues with AI agents
In @.github/scripts/generate-package-update-report.cjs:
- Around line 26-33: The current pattern in the package extraction block (where
updatedPackages is populated) is too loose and picks up non-package keys like
"__metadata"; update the parsing to only consider yarn.lock entry header lines
that end with a colon and then extract the package descriptor/name from that
header (i.e., match lines like 'package@version:'), rejecting keys without
trailing ':'; modify the two regex checks around the line.match(...) and the
descriptor extraction (the match(...) used to populate updatedPackages) to first
assert the line ends with ':' and then capture the package identifier (including
scoped names and potential slash) from the descriptor before adding to
updatedPackages.
In @.github/workflows/docker-publish.yml:
- Around line 3-4: The workflow currently only uses the workflow_dispatch
trigger, so automatic Docker builds on version tag pushes were removed; restore
the push.tags trigger by updating the workflow's triggers to include a push
event filtering tags (e.g., add a push: tags: ['v*'] entry alongside
workflow_dispatch) so that when version tags like "v1.7.0" are pushed the Docker
publish job runs automatically; modify the "on:" block in
.github/workflows/docker-publish.yml (where workflow_dispatch is declared) to
include the push.tags pattern.
In @.github/workflows/phpstan.yml:
- Around line 65-93: The "Comment on PR (if available)" step is unreachable
because the workflow only triggers on workflow_dispatch while the step checks
github.event_name == 'pull_request'; either remove this step entirely if PR
comments aren't needed, or update the workflow triggers to include pull_request
so the condition can be true — adjust the top-level "on:" entries to add
pull_request (or change the step's if to match an existing trigger) and keep the
step's existing fs/readFileSync and github.rest.issues.createComment logic
intact.
In @.github/workflows/pint.yml:
- Around line 14-18: Update the pull_request trigger filter under the workflow's
pull_request block so it includes the repository's actual default branch name;
replace or add the correct branch 'development' alongside 'master' and any other
branches listed (currently 'develop') so the Pint workflow runs for PRs
targeting 'development' — modify the branches array in the pull_request YAML
block to include 'development' (and remove 'develop' if that was a typo).
- Around line 93-101: The push step "Commit and Push Changes" should skip when
the PR comes from a fork; update its if condition to only run when changes exist
and the workflow is running in the same repository (not a fork). Replace the
current condition (steps.verify_diff.outputs.has_changes == 'true') with a
compound check such as: steps.verify_diff.outputs.has_changes == 'true' &&
(github.event.pull_request == null ||
github.event.pull_request.head.repo.full_name == github.repository) so the push
runs for non-PR runs or PRs originating from the same repo but is skipped for
forked PRs.
In @.github/workflows/quickstart.yml:
- Around line 59-68: The workflow currently runs Laravel-only commands (steps
named "Prepare Laravel environment", "Run database migrations", "Run smoke
tests") using `php artisan`, which don't exist in this CodeIgniter 3.x repo;
remove or replace those steps: either delete this workflow if no CI tests are
needed, or update the steps to CodeIgniter-compatible actions such as installing
PHP dependencies (composer install), copying the repository's CodeIgniter
env/example file into the runtime env, setting up any DB fixtures/migrations
using the project's migration script or SQL seed files, and running tests via
the project's configured test runner (e.g., phpunit) instead of `php artisan
test`; adjust step names ("Prepare Laravel environment", "Run database
migrations", "Run smoke tests") accordingly so they reflect CodeIgniter
commands.
In @.github/workflows/README.md:
- Around line 623-644: Remove the hard-coded line numbers in the README and
update the instructions to reference searchable identifiers instead: instruct
users to edit the release.yml keys (php-version, node-version, retention-days)
by locating those keys in release.yml and to modify the "Create release zip"
step by its step name ("Create release zip") when changing ZIP exclusions;
replace each outdated line-number instruction with guidance to find and edit the
specific key or step name so the docs remain correct across file edits.
- Line 40: Update the README line that currently says "Automatically runs on
every push to the `master` branch" to accurately describe the actual trigger
used by the workflow: reference the workflow file (release.yml) and state it
uses workflow_dispatch (manual trigger) and requires selecting a release type;
replace the sentence accordingly so it mentions manual dispatch and release type
selection rather than automatic pushes to master.
In @.github/workflows/setup.yml:
- Around line 34-51: Update the workflow input descriptions to remove "php
artisan" and reference CodeIgniter's CLI instead: replace the description string
'Run php artisan key:generate' with something like 'Run CodeIgniter CLI (php
spark) key generation' (or 'Run php spark key:generate'), change 'Run php
artisan migrate' (input run_migrate) to 'Run CodeIgniter migration (php spark
migrate)' and change 'Run php artisan db:seed' (input run_seed) to 'Run
CodeIgniter seeder (php spark db:seed)'; ensure you only update the description
values for these inputs so the dispatch UI labels are accurate for a CodeIgniter
workflow.
In @.github/workflows/test-frontend.yml:
- Around line 6-7: The workflow currently only uses the on: workflow_dispatch
trigger which disables automatic PR testing; update the on: section to include
pull_request (e.g., change from only workflow_dispatch to on: { pull_request:
{}, workflow_dispatch: {} }) so frontend tests run automatically on PRs as well
as allowing manual dispatch; ensure the pull_request entry is present and
configured with any branch filters you need.
In `@application/modules/guest/controllers/gateways/Stripe.php`:
- Line 122: The code is accessing nonexistent Checkout Session properties
($session->cancel, $session->cancellation_reason, $session->last_payment_error);
update the logic that builds the error/explanation string to use
$session->status or $session->payment_status instead and remove uses of
$session->cancel/cancellation_reason, and when you need detailed payment errors
expand or retrieve the session with 'payment_intent' and read
$session->payment_intent->last_payment_error as the source of payment failure
info; adjust the fallback plumbing so the expression that previously referenced
($session->cancel ? $session->cancellation_reason :
$session->last_payment_error) instead checks status/payment_status and, if
necessary, uses the expanded payment_intent->last_payment_error for detailed
messages.
In `@application/modules/invoices/views/view.php`:
- Around line 468-492: These three text inputs (invoice_quote_number,
invoice_work_order, invoice_agreement) are editable even when the invoice is
read-only; update their markup to emit the same read-only/disabled attribute
used by other invoice fields by conditionally adding readonly/disabled when the
invoice is read-only (use the same condition/variable other fields use, e.g.
$invoice->is_read_only or the existing invoice-status check), so the inputs are
non-editable in read-only state.
In `@application/modules/quotes/controllers/Ajax.php`:
- Around line 49-62: The modal_quote_lookups_select method currently returns
nothing when $quote_id is empty which breaks client-side JSON.parse; update
modal_quote_lookups_select to validate $quote_id and return a consistent JSON
error response (and appropriate HTTP status) instead of silence — e.g., when
$quote_id is empty respond with a JSON object like {"error":"quote_id required"}
using the same output mechanism used elsewhere (use
$this->output->set_content_type('application/json')->set_output(json_encode(...))
or equivalent) so callers parsing JSON won't fail; keep the existing logic that
loads $this->mdl_quotes and returns the quote when $quote_id is present.
In `@application/modules/quotes/views/modal_quote_lookups.php`:
- Around line 101-103: The current echo of
$quote_statuses[$quote->quote_status_id]['label'] can raise an undefined index
when $quote->quote_status_id is missing or not present in $quote_statuses;
update the view to check that isset($quote_statuses[$quote->quote_status_id])
(and optionally isset(['label'])) before accessing it and render a safe fallback
(e.g. a default label like "Unknown" or an empty string) when the lookup fails
so the template doesn't trigger notices; locate the usage in
modal_quote_lookups.php where $quote_statuses and $quote->quote_status_id are
referenced and add the conditional fallback logic there.
In `@application/modules/sessions/controllers/Sessions.php`:
- Around line 323-444: The current session-based rate limiting in
_is_ip_rate_limited_password_reset, _record_password_reset_attempt,
_is_email_rate_limited_password_reset and _record_email_password_reset_attempt
is bypassable by clearing cookies; replace session storage with a server-side
store (Redis or DB) keyed by IP/email with a TTL (e.g. keys like
password_reset:ip:{md5(ip)} and password_reset:email:{md5(email)}), implement
atomic increment or append-with-expiry and window-based trimming on read,
enforce counts against env-configured limits, persist attempts with expiry
instead of session->userdata/session->set_userdata, and update logging and tests
to reflect the new storage mechanism.
In `@application/modules/setup/sql/038_1.6.1.sql`:
- Around line 1-6: The migration duplicates columns already added in
041_1.7.0.sql causing "Duplicate column name" errors; update 038_1.6.1.sql to
avoid re-adding the same columns by either removing the six ALTER TABLE
statements for client_number (ip_clients), invoice_quote_number,
invoice_work_order, invoice_agreement (ip_invoices), and quote_work_order,
quote_agreement (ip_quotes), or replace each direct ALTER with conditional logic
that first checks for the column's existence (e.g., query
INFORMATION_SCHEMA/COLUMN_NAME or use CREATE/ALTER ... IF NOT EXISTS where
supported) and only runs the ALTER if the column is missing; ensure the chosen
fix preserves the intended data type/position consistency with 041_1.7.0.sql
(VARCHAR(255)/TEXT and AFTER positions) so sequential upgrades do not conflict.
In `@application/modules/setup/sql/041_1.7.0.sql`:
- Around line 1-17: The migrations create the same columns with conflicting
types—ensure a single canonical type and remove duplicate creations: update
041_1.7.0.sql to use the same types as 038_1.6.1.sql (set client_number,
invoice_quote_number, invoice_work_order, invoice_agreement, quote_work_order,
quote_agreement to VARCHAR(100) instead of VARCHAR(255)/TEXT) or conversely
consolidate by removing the duplicate column ALTERs from 038_1.6.1.sql and
standardizing on VARCHAR(255)/TEXT across both files; pick one schema, update
the ALTER TABLE statements in ip_clients/ip_invoices/ip_quotes (referencing
client_number, invoice_quote_number, invoice_work_order, invoice_agreement,
quote_work_order, quote_agreement) to that type, and delete the
duplicate/contradictory ALTER in the other migration so only one migration adds
each column.
In `@assets/core/js/scripts.js`:
- Around line 76-114: The anchor-href regex in sanitizeHtml incorrectly uses a
character class for " which breaks matching; update the replacement that
handles <a> tags (the escaped variable replacement block) to use a non-greedy
capture like /<a\s+href="(.*?)">/gi (keeping the same href
validation logic and callback signature), so the entire href value between the
escaped "..." is captured reliably; ensure you update the
corresponding replace call on escaped (inside sanitizeHtml) to use that new
regex and preserve the existing URL validation checks.
In `@package.json`:
- Around line 38-40: package.json is pinning "sass": "^1.97" which introduces
breaking changes; before upgrading, search the codebase for deprecated/changed
Sass patterns and update them: locate any "@function type" definitions and
rename them, replace all "@import" usages with "@use" / "@forward" modules,
remove or replace "meta.feature-exists()" calls, audit any "@use"/"@forward ...
with" that depend on private variables and convert to public APIs, and scan for
nested rules/declarations whose emitted order may change and adjust
selectors/mixins accordingly; run the test build (and compare generated CSS)
after updating to validate outputs and adjust styles where order-dependent
behavior changed.
♻️ Duplicate comments (3)
assets/core/js/jquery-ui.js (1)
888-894: Security fix for XSS vulnerability correctly applied.This change properly mitigates CVE-2021-41184 by using
$(document).find(options.of)for string inputs, ensuring they're treated strictly as selectors rather than potentially interpreted as HTML. Theelsebranch correctly handles non-string types (DOM elements, jQuery objects, Events, window/document) which are safe to pass directly to$().As noted in previous reviews, consider upgrading to jQuery UI 1.13.0+ which includes this fix natively, reducing the maintenance burden of vendored patches.
application/views/quote_templates/public/InvoicePlane_Web.php (1)
114-123: Escape client VAT/tax IDs and attachment names to prevent stored XSS.These fields can contain user-supplied values and should be HTML-escaped before output.
🛠️ Suggested fix
- echo ': ' . $quote->client_vat_id . '<br>'; + echo ': ' . htmlsc($quote->client_vat_id) . '<br>'; ... - echo ': ' . $quote->client_tax_code . '<br>'; + echo ': ' . htmlsc($quote->client_tax_code) . '<br>'; ... - <td><?php echo $attachment['name']; ?></td> + <td><?php echo htmlsc($attachment['name']); ?></td>Also applies to: 315-317
.github/workflows/composer-update.yml (1)
86-92: Smoke-test step still calls Laravel Artisan.If this is CodeIgniter,
artisanwon’t exist and this step will fail or be skipped unintentionally. Please switch to the project’s actual test runner or guard the command.🛠️ Safer guard (replace with project-specific command as needed)
- php artisan key:generate --env=testing - php artisan test --configuration=phpunit.smoke.xml --env=testing + if [ -f artisan ]; then + php artisan key:generate --env=testing + php artisan test --configuration=phpunit.smoke.xml --env=testing + elif [ -x vendor/bin/phpunit ]; then + vendor/bin/phpunit --configuration=phpunit.smoke.xml + else + echo "::warning::No test runner found; skipping smoke tests." + fi#!/bin/bash # Verify whether Laravel's artisan or a phpunit runner exists ls -la artisan 2>/dev/null || true fd -a 'phpunit\.smoke\.xml|phpunit\.xml' . fd -a 'phpunit' vendor/bin 2>/dev/null || true
🧹 Nitpick comments (9)
application/modules/settings/models/Mdl_versions.php (1)
44-44: Prefer explicitreturn null;for clarity.Using
return;implicitly returnsnull, but an explicitreturn null;would be more consistent with the docblock (@return string|null) and makes the intent clearer for maintainability.✨ Suggested change
if ($result->query->num_rows() === 0) { - return; + return null; }.github/workflows/test-frontend.yml (1)
14-15: Node 14 and 16 are end-of-life."Production applications should only use Active LTS or Maintenance LTS releases." Node.js 18 maintenance LTS ends April 2025, Node.js 20 ends April 2026, and Node.js 22 ends April 2027. Node 14 and 16 reached EOL in April 2023 and September 2023 respectively.Testing against EOL versions provides limited value and may produce false positives on deprecated APIs.
♻️ Suggested update to supported Node.js versions
matrix: operating-system: [ubuntu-latest] - node: ['14', '16', '18'] + node: ['20', '22']application/helpers/client_helper.php (1)
49-56: Add null safety check for$clientparameter.Accessing
$client->client_numberwithout checking if$clientis null could cause an error when called with invalid data. Also, consider using strict comparison orempty()for consistency with the rest of the codebase.♻️ Suggested improvement
function format_client_number($client) { - if ($client->client_number != "") { + if (!empty($client) && !empty($client->client_number)) { return $client->client_number; } return "00-00000"; }.github/workflows/docker.yml (2)
19-32: Consider adding a cleanup step to ensure containers are stopped.The workflow builds and starts containers but doesn't clean them up. While GitHub-hosted runners are ephemeral, adding a cleanup step is a good practice for consistency and in case the workflow is ever run on self-hosted runners.
♻️ Suggested cleanup step
- name: Check running services run: docker-compose ps `#-` name: # Optional: Run health checks or tests # run: # echo "TODO: insert test commands after containers are up" + + - name: Cleanup + if: always() + run: docker-compose down --volumes --remove-orphans
23-24: Usedocker composeinstead of the deprecateddocker-composecommand.Docker Compose V2 is integrated into Docker CLI and the standalone
docker-composecommand is deprecated. Docker recommends usingdocker compose(with a space) for forward compatibility and to benefit from active maintenance. Thedocker-composecommand continues to work through aliasing but will receive only critical and security fixes.Consider updating line 24 from
docker-compose up -d --buildtodocker compose up -d --build..github/workflows/phpstan.yml (1)
25-30: Consider capturing PHPStan exit code separately.When PHPStan finds errors, it exits non-zero, but
continue-on-error: truemasks this. The subsequent "Check for errors" step (lines 50-54) correctly fails, but capturing the exit code explicitly would be cleaner and more debuggable.Suggested improvement
- name: Run PHPStan (JSON output) id: phpstan-analysis run: | - vendor/bin/phpstan analyse --memory-limit=1G --error-format=json > phpstan.json - cat phpstan.json + set +e + vendor/bin/phpstan analyse --memory-limit=1G --error-format=json > phpstan.json + echo "exit_code=$?" >> $GITHUB_OUTPUT + set -e + cat phpstan.json continue-on-error: trueapplication/modules/quotes/models/Mdl_quotes.php (1)
345-361: Use short array syntax for consistency.
The rest of the model already uses[].♻️ Suggested refactor
- $this->filter_where_in('quote_status_id', array(2, 3, 4)); + $this->filter_where_in('quote_status_id', [2, 3, 4]);application/modules/quotes/views/view.php (1)
347-359: XSS protection correctly applied.The
htmlsc()function properly escapes output values. The null coalescing (?? '') handles missing values gracefully.For improved accessibility, consider adding
forattributes to the labels to formally associate them with the inputs.♿ Accessibility improvement
<div class="col-xs-12 col-md-4"> <div class="quote-properties"> - <label><?php _trans('work_order'); ?></label> + <label for="quote_work_order"><?php _trans('work_order'); ?></label> <input type="text" id="quote_work_order" class="form-control input-sm" value="<?php echo htmlsc($quote->quote_work_order ?? ''); ?>"> </div> <div class="quote-properties"> - <label><?php _trans('agreement'); ?></label> + <label for="quote_agreement"><?php _trans('agreement'); ?></label> <input type="text" id="quote_agreement" class="form-control input-sm" value="<?php echo htmlsc($quote->quote_agreement ?? ''); ?>"> </div> </div>application/modules/quotes/controllers/Ajax.php (1)
21-47: Previous undefined$dataissue resolved.The
$dataarray is now properly initialized with default values (lines 27-32) before conditional population.However, line 37 calls
$this->mdl_quotes->statuses()without using the return value - this call is redundant sincestatuses()is called again on line 40.♻️ Remove redundant call
if (!empty($client_id)) { $this->mdl_quotes->by_client($client_id); $this->mdl_quotes->related_is_open(); - $this->mdl_quotes->statuses(); $quotes = $this->mdl_quotes->get()->result(); $quote_statuses = $this->mdl_quotes->statuses();
Comment on lines
323
to
444
| /** | ||
| * Check if IP address has exceeded rate limit for password resets using session storage | ||
| * Check if IP address has exceeded rate limit for password resets using session storage. | ||
| * | ||
| * @param int $max_attempts Maximum attempts allowed per hour | ||
| * @param int $max_attempts Maximum attempts allowed per hour | ||
| * @param int $window_minutes Time window in minutes | ||
| * | ||
| * @return bool True if rate limited, false otherwise | ||
| */ | ||
| private function _is_ip_rate_limited_password_reset() | ||
| { | ||
| $max_attempts = env('PASSWORD_RESET_IP_MAX_ATTEMPTS', 5); | ||
| $max_attempts = env('PASSWORD_RESET_IP_MAX_ATTEMPTS', 5); | ||
| $window_minutes = env('PASSWORD_RESET_IP_WINDOW_MINUTES', 60); | ||
| $ip_address = $this->input->ip_address(); | ||
|
|
||
| $ip_address = $this->input->ip_address(); | ||
| $session_key = 'password_reset_attempts_' . md5($ip_address); | ||
|
|
||
| // Get current attempts from session | ||
| $attempts = $this->session->userdata($session_key); | ||
| if (!$attempts) { | ||
|
|
||
| if ( ! $attempts) { | ||
| $attempts = []; | ||
| } | ||
|
|
||
| // Clean up old attempts outside the time window | ||
| $cutoff_time = time() - ($window_minutes * 60); | ||
| $attempts = array_filter($attempts, function($timestamp) use ($cutoff_time) { | ||
| $attempts = array_filter($attempts, function ($timestamp) use ($cutoff_time) { | ||
| return $timestamp > $cutoff_time; | ||
| }); | ||
|
|
||
| // Check if rate limited | ||
| if (count($attempts) >= $max_attempts) { | ||
| log_message('info', trans('log_ip_rate_limit_check') . ': ' . count($attempts) . ' attempts from IP: ' . $ip_address); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Record a password reset attempt for the current IP | ||
| * Record a password reset attempt for the current IP. | ||
| */ | ||
| private function _record_password_reset_attempt() | ||
| { | ||
| $ip_address = $this->input->ip_address(); | ||
| $ip_address = $this->input->ip_address(); | ||
| $session_key = 'password_reset_attempts_' . md5($ip_address); | ||
|
|
||
| // Get current attempts from session | ||
| $attempts = $this->session->userdata($session_key); | ||
| if (!$attempts) { | ||
|
|
||
| if ( ! $attempts) { | ||
| $attempts = []; | ||
| } | ||
|
|
||
| // Add current timestamp | ||
| $attempts[] = time(); | ||
|
|
||
| // Store back to session | ||
| $this->session->set_userdata($session_key, $attempts); | ||
| } | ||
|
|
||
| /** | ||
| * Check if email-based rate limit exceeded for password resets using session storage | ||
| * Check if email-based rate limit exceeded for password resets using session storage. | ||
| * | ||
| * @param string $email Email address to check | ||
| * @param int $max_attempts Maximum attempts allowed | ||
| * @param int $window_hours Time window in hours | ||
| * @param string $email Email address to check | ||
| * @param int $max_attempts Maximum attempts allowed | ||
| * @param int $window_hours Time window in hours | ||
| * | ||
| * @return bool True if rate limited, false otherwise | ||
| */ | ||
| private function _is_email_rate_limited_password_reset($email) | ||
| { | ||
| $max_attempts = env('PASSWORD_RESET_EMAIL_MAX_ATTEMPTS', 3); | ||
| $window_hours = env('PASSWORD_RESET_EMAIL_WINDOW_HOURS', 1); | ||
|
|
||
| $session_key = 'password_reset_email_' . md5($email); | ||
|
|
||
| // Get current attempts from session | ||
| $attempts = $this->session->userdata($session_key); | ||
| if (!$attempts) { | ||
|
|
||
| if ( ! $attempts) { | ||
| $attempts = []; | ||
| } | ||
|
|
||
| // Clean up old attempts outside the time window | ||
| $cutoff_time = time() - ($window_hours * 3600); | ||
| $attempts = array_filter($attempts, function($timestamp) use ($cutoff_time) { | ||
| $attempts = array_filter($attempts, function ($timestamp) use ($cutoff_time) { | ||
| return $timestamp > $cutoff_time; | ||
| }); | ||
|
|
||
| // Check if rate limited | ||
| if (count($attempts) >= $max_attempts) { | ||
| log_message('info', trans('log_email_rate_limit_check') . ': ' . count($attempts) . ' attempts for email: ' . $email); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Record a password reset attempt for a specific email | ||
| * Record a password reset attempt for a specific email. | ||
| * | ||
| * @param string $email Email address | ||
| */ | ||
| private function _record_email_password_reset_attempt($email) | ||
| { | ||
| $session_key = 'password_reset_email_' . md5($email); | ||
|
|
||
| // Get current attempts from session | ||
| $attempts = $this->session->userdata($session_key); | ||
| if (!$attempts) { | ||
|
|
||
| if ( ! $attempts) { | ||
| $attempts = []; | ||
| } | ||
|
|
||
| // Add current timestamp | ||
| $attempts[] = time(); | ||
|
|
||
| // Store back to session | ||
| $this->session->set_userdata($session_key, $attempts); | ||
| } |
Contributor
There was a problem hiding this comment.
Session-scoped reset limits are bypassable.
Because attempts are stored in the session, a requester can clear cookies/start a new session and reset the counters. For password resets, this defeats rate limiting. Consider persisting attempts server-side (DB/cache) keyed by IP/email with TTL.
🤖 Prompt for AI Agents
In `@application/modules/sessions/controllers/Sessions.php` around lines 323 -
444, The current session-based rate limiting in
_is_ip_rate_limited_password_reset, _record_password_reset_attempt,
_is_email_rate_limited_password_reset and _record_email_password_reset_attempt
is bypassable by clearing cookies; replace session storage with a server-side
store (Redis or DB) keyed by IP/email with a TTL (e.g. keys like
password_reset:ip:{md5(ip)} and password_reset:email:{md5(email)}), implement
atomic increment or append-with-expiry and window-based trimming on read,
enforce counts against env-configured limits, persist attempts with expiry
instead of session->userdata/session->set_userdata, and update logging and tests
to reflect the new storage mechanism.
Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/8c81a4f8-e3a1-4c1d-86c4-585cf1c118fd Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…afety, dead code, DOM sanitizer Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/bf2724e4-678b-4ed5-9c24-3a3758d28493 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…er entry point Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/bf2724e4-678b-4ed5-9c24-3a3758d28493 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/bf2724e4-678b-4ed5-9c24-3a3758d28493 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…ed as HTML' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ate CHANGELOG and UPGRADE guide Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/d48e22c6-b03a-42e2-86e9-941490936d14 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…ENTS.md Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/f9be7229-f85e-423e-9683-9ea095e6bd74 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…ADME Agent-Logs-Url: https://github.com/InvoicePlane/InvoicePlane/sessions/ef8eee14-a7f4-4753-be32-4afd75e6a563 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
nielsdrost7
force-pushed
the
1022-add-extra-fields-and-functions-to-client-and-quotes-and-invoices
branch
2 times, most recently
from
113d55e to
e8558ef
Compare
nielsdrost7
force-pushed
the
1022-add-extra-fields-and-functions-to-client-and-quotes-and-invoices
branch
from
e8558ef to
5c41820
Compare
Contributor
|
Hey @novafirst can you re-do your feature please? The code probably in Controllers. Models, etc, didn't survive and therefore the feature isn't complete. My bad! my apologies. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added a few new fields and additional functions to clients, invoices and quotes modules
List of all new fields:
client_number
invoice_quote_number
invoice_work_order
invoice_agreement
quote_work_order
quote_agreement
Related Issue
https://community.invoiceplane.com/t/topic/10662
Motivation and Context
This is an improvement to already great application. Having ability to have client_number for each client is sometimes mandatory for a larger size company or when billing requires that number.
So in Clients module, added one additional field:
client_number
For Invoices three additional fields and a new function:
invoice_quote_number
invoice_work_order
invoice_agreement
invoice_quote_number -> This is a text field, you can type in a related quote number or using a new function you can select related quote from a list by clicking on Change Related Quote. This will associate the invoice with specific quote and import quote_work_order and quote_agreement values (if any) from the quotes and pass it to invoice_work_order and invoice_agreement.
For Quotes two additional fields:
quote_work_order
quote_agreement
Welcome to any suggestions!
Screenshots (if appropriate):
Pull Request Checklist
Issue Type (Please check one or more)
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.