Skip to content
Open

Dev #73

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 96 additions & 23 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ jobs:
DB_USERNAME: root
DB_PASSWORD: phplist
BROADCAST_DRIVER: log
API_BASE_URL: http://api.phplist.local/
REST_API_BASE_URL: http://api.phplist.local/api/v2
services:
mysql:
image: mysql:5.7
Expand All @@ -31,7 +33,20 @@ jobs:
with:
php-version: ${{ matrix.php-versions }}
extensions: mbstring, dom, fileinfo, mysql
coverage: xdebug #optional
coverage: xdebug

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 20
cache: yarn

- name: Install JS dependencies
run: yarn install --frozen-lockfile

- name: Build frontend assets
run: yarn encore dev

- name: Install Symfony CLI
run: |
curl -sS https://get.symfony.com/cli/installer | bash
Expand All @@ -43,14 +58,11 @@ jobs:
echo "deb [arch=amd64 signed-by=/usr/share/keyrings/google.gpg] http://dl.google.com/linux/chrome/deb/ stable main" | sudo tee /etc/apt/sources.list.d/google-chrome.list
sudo apt-get update
sudo apt-get install -y google-chrome-stable
- name: Set Panther to use Chrome
run: |
echo "PANTHER_NO_HEADLESS=0" >> .env.test
echo "PANTHER_CHROME_BINARY=/usr/bin/google-chrome" >> .env.test
- name: Start mysql service
run: sudo /etc/init.d/mysql start
sudo apt install socat
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make the socat install non-interactive.

sudo apt install socat can prompt for confirmation in CI. Use apt-get install -y to keep the workflow deterministic.

🔧 Proposed fix
-          sudo apt install socat
+          sudo apt-get install -y socat
📝 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
sudo apt install socat
sudo apt-get install -y socat
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml at line 61, Replace the interactive apt install
invocation in the CI workflow: change the line that runs "sudo apt install
socat" to a non-interactive installation using apt-get with the -y flag and
include an apt-get update beforehand (e.g., use apt-get update && apt-get
install -y socat) so the CI won't prompt for confirmation.


- name: Verify MySQL connection on host
run: mysql --host 127.0.0.1 --port ${{ job.services.mysql.ports['3306'] }} -u${{ env.DB_USERNAME }} -p${{ env.DB_PASSWORD }} -e "SHOW DATABASES"

- name: Get composer cache directory
id: composer-cache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
Expand All @@ -60,21 +72,82 @@ jobs:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
restore-keys: ${{ runner.os }}-composer-
- name: Install the latest dependencies
run: composer install
- name: Install PHP dependencies
run: composer install --no-interaction --prefer-dist

- name: Set up database schema
run: mysql --host 127.0.0.1 --port ${{ job.services.mysql.ports['3306'] }} -u${{ env.DB_USERNAME }} -p${{ env.DB_PASSWORD }} ${{ env.DB_DATABASE }} < vendor/phplist/core/resources/Database/Schema.sql
- name: Validating composer.json
run: composer validate --no-check-all --no-check-lock --strict;
- name: Linting all php files
run: find src/ tests/ public/ -name ''*.php'' -print0 | xargs -0 -n 1 -P 4 php -l; php -l;
- name: Running integration tests with phpunit
run: vendor/bin/phpunit tests/Integration/;
- name: Running the system tests
run: vendor/bin/phpunit tests/System/;
- name: Running static analysis
run: vendor/bin/phpstan analyse -l 5 src/ tests/;
- name: Running PHPMD
run: vendor/bin/phpmd src/ text vendor/phplist/core/config/PHPMD/rules.xml;
- name: Running PHP_CodeSniffer
run: vendor/bin/phpcs --standard=vendor/phplist/core/config/PhpCodeSniffer/ src/ tests/;
- name: Validate composer.json
run: composer validate --no-check-all --no-check-lock --strict
- name: Lint PHP files
run: find src/ tests/ public/ -name '*.php' -print0 | xargs -0 -n 1 -P 4 php -l

- name: Run static analysis
run: vendor/bin/phpstan analyse -l 5 src/ tests/
- name: Run PHPMD
run: vendor/bin/phpmd src/ text vendor/phplist/core/config/PHPMD/rules.xml
- name: Run PHP_CodeSniffer
run: vendor/bin/phpcs --standard=vendor/phplist/core/config/PhpCodeSniffer/ src/ tests/

- name: Install Prism
run: npm install -g @stoplight/prism-cli
- name: Start Prism Mock Server
run: |
prism mock --host 127.0.0.1 --port 4010 ./openapi.json &
- name: Add local hostname
run: echo "127.0.0.1 api.phplist.local" | sudo tee -a /etc/hosts
- name: Proxy port 80 to 4010
run: |
sudo socat -d -d TCP-LISTEN:80,reuseaddr,fork TCP:127.0.0.1:4010 &
- name: Wait for Prism and proxy
run: |
set -euo pipefail
prism_ready=0
for i in $(seq 1 30); do
if curl -sS -o /tmp/prism-health-body.txt -w "%{http_code}" \
-H 'Content-Type: application/json' \
-X POST http://127.0.0.1:4010/api/v2/sessions \
--data '{"login_name":"healthcheck","password":"healthcheck"}' > /tmp/prism-health-code.txt; then
code=$(cat /tmp/prism-health-code.txt)
if [ "$code" != "000" ]; then
echo "Prism is reachable on 127.0.0.1:4010 with HTTP ${code}"
prism_ready=1
break
fi
fi
sleep 1
done
if [ "$prism_ready" -ne 1 ]; then
echo "Prism did not become reachable in time."
exit 1
fi

proxy_ready=0
for i in $(seq 1 30); do
if curl -sS -o /tmp/proxy-health-body.txt -w "%{http_code}" \
-H 'Content-Type: application/json' \
-X POST http://api.phplist.local/api/v2/sessions \
--data '{"login_name":"healthcheck","password":"healthcheck"}' > /tmp/proxy-health-code.txt; then
code=$(cat /tmp/proxy-health-code.txt)
if [ "$code" != "000" ]; then
echo "Proxy is reachable on api.phplist.local with HTTP ${code}"
proxy_ready=1
break
fi
fi
sleep 1
done
if [ "$proxy_ready" -ne 1 ]; then
echo "Proxy did not become reachable in time."
exit 1
fi

- name: Run tests with phpunit
run: vendor/bin/phpunit tests

- name: Upload Panther screenshots
if: failure()
uses: actions/upload-artifact@v4
with:
name: panther-screenshots
path: var/screenshots
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ Please install this package via Composer from within the
which also has more detailed installation instructions in the README.


### When this module is installed as a dependency, publish bundle assets to the host application's `public/` directory:
```bash
php bin/console assets:install public --symlink --relative
```

This module serves its frontend files from `/`.


## Contributing to this package

Please read the [contribution guide](.github/CONTRIBUTING.md) on how to
Expand Down
26 changes: 26 additions & 0 deletions apache/web-frontend.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<VirtualHost *:80>
ServerName frontend.phplist.local
ServerAdmin webmaster@localhost

DocumentRoot /{pathToTheProject}/web-frontend/public

ErrorLog ${APACHE_LOG_DIR}/web-frontend-error.log
CustomLog ${APACHE_LOG_DIR}/web-frontend-access.log combined

<Directory /{pathToTheProject}/web-frontend/public>
Options FollowSymLinks
AllowOverride All
Require all granted
DirectoryIndex app.php
</Directory>

<FilesMatch "^\.">
Require all denied
</FilesMatch>

<IfModule proxy_fcgi_module>
<FilesMatch \.php$>
SetHandler "proxy:unix:/run/php/php8.1-fpm.sock|fcgi://localhost"
</FilesMatch>
</IfModule>
</VirtualHost>
8 changes: 6 additions & 2 deletions assets/app.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import './styles/app.css';
import { createApp } from 'vue';
import App from './vue/App.vue';
import { router } from './router';

// Mount the main app if the element exists
const appElement = document.getElementById('vue-app');

if (appElement) {
createApp(App).mount('#vue-app');
const app = createApp(App);
app.use(router);
app.mount(appElement);
}

Binary file added assets/images/avatar.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/logo-48.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/logo.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions assets/router/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { createRouter, createWebHistory } from 'vue-router';
import DashboardView from '../vue/views/DashboardView.vue'
import SubscribersView from '../vue/views/SubscribersView.vue'
import ListsView from '../vue/views/ListsView.vue'
import ListSubscribersView from '../vue/views/ListSubscribersView.vue'
import CampaignsView from '../vue/views/CampaignsView.vue'
import CampaignEditView from '../vue/views/CampaignEditView.vue'
import TemplatesView from '../vue/views/TemplatesView.vue'
import TemplateEditView from '../vue/views/TemplateEditView.vue'

export const router = createRouter({
history: createWebHistory(),
routes: [
{ path: '/', name: 'dashboard', component: DashboardView, meta: { title: 'Dashboard' } },
{ path: '/subscribers', name: 'subscribers', component: SubscribersView, meta: { title: 'Subscribers' } },
{ path: '/lists', name: 'lists', component: ListsView, meta: { title: 'Lists' } },
{ path: '/campaigns', name: 'campaigns', component: CampaignsView, meta: { title: 'Campaigns' } },
{ path: '/templates', name: 'templates', component: TemplatesView, meta: { title: 'Templates' } },
{ path: '/templates/create', name: 'template-create', component: TemplateEditView, meta: { title: 'Create Template' } },
{ path: '/templates/:templateId/edit', name: 'template-edit', component: TemplateEditView, meta: { title: 'Edit Template' } },
{ path: '/campaigns/create', name: 'campaign-create', component: CampaignEditView, meta: { title: 'Create Campaign' } },
{ path: '/campaigns/:campaignId/edit', name: 'campaign-edit', component: CampaignEditView, meta: { title: 'Edit Campaign' } },
{ path: '/lists/:listId/subscribers', name: 'list-subscribers', component: ListSubscribersView, meta: { title: 'List Subscribers' } },
{ path: '/:pathMatch(.*)*', redirect: '/' },
],
});

router.afterEach((to) => {
const defaultTitle = 'phpList';
const pageTitle = to.meta.title;
document.title = pageTitle ? `${defaultTitle} - ${pageTitle}` : defaultTitle;
});
Comment on lines +22 to +32
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Confirm campaign edit title mismatch between backend title and router meta title.
rg -nP "Edit Campaign|to\.meta\.title|document\.title|campaignId" -C3 assets src tests

Repository: phpList/web-frontend

Length of output: 23299


Make the campaign ID visible in the edit page title.

The backend renders Edit Campaign #123`` for /campaigns/123/edit, but the router's static `meta.title` overwrites it with just `Edit Campaign` after navigation, losing the ID. The Vue component already computes the correct title internally, so the router hook needs to support dynamic meta titles.

The proposed fix—making meta.title a function that accepts the route and checking for it in the router.afterEach hook—will solve this.

Proposed fix
-        { path: '/campaigns/:campaignId/edit', name: 'campaign-edit', component: CampaignEditView, meta: { title: 'Edit Campaign' } },
+        {
+            path: '/campaigns/:campaignId/edit',
+            name: 'campaign-edit',
+            component: CampaignEditView,
+            meta: { title: (route) => `Edit Campaign #${route.params.campaignId}` },
+        },
@@
 router.afterEach((to) => {
     const defaultTitle = 'phpList';
-    const pageTitle = to.meta.title;
+    const pageTitle = typeof to.meta.title === 'function' ? to.meta.title(to) : to.meta.title;
     document.title = pageTitle ? `${defaultTitle} - ${pageTitle}` : defaultTitle;
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/router/index.js` around lines 17 - 27, The router currently overwrites
dynamic titles because meta.title is a static string for the route
'/campaigns/:campaignId/edit' (name: 'campaign-edit') and router.afterEach
always uses to.meta.title; change meta.title to accept either a string or a
function and update the router.afterEach hook (router.afterEach) to detect if
to.meta.title is a function and, if so, call it with the current route (to) to
get the pageTitle (falling back to the string value or defaultTitle) so
CampaignEditView can surface the computed "Edit Campaign `#123`" title without
losing support for static titles.

16 changes: 16 additions & 0 deletions assets/styles/app.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@import "tailwindcss";
@tailwind utilities;
Comment on lines +1 to +2
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

How to configure Stylelint to ignore Tailwind CSS v4 at-rules like @themeand@source?

💡 Result:

To configure Stylelint to ignore Tailwind CSS v4 at-rules like @theme and @source, update your Stylelint configuration file (e.g., stylelint.config.js) to override the "at-rule-no-unknown" rule and list the Tailwind v4 at-rules in the ignoreAtRules array. Example configuration: js module.exports = { extends: [ 'stylelint-config-standard', // or your preferred base config ], rules: { 'at-rule-no-unknown': [ true, { ignoreAtRules: [ 'theme', 'source', 'apply', 'utility', 'variant', 'plugin', 'config', 'reference', 'custom-variant' ] } ] } }; If using SCSS, also disable the base rule and configure the SCSS-specific one: js rules: { 'at-rule-no-unknown': null, 'scss/at-rule-no-unknown': [ true, { ignoreAtRules: [ 'theme', 'source', // ... other rules ] } ] } Optionally, install and extend a Tailwind-specific config like stylelint-config-tailwindcss (npm install --save-dev stylelint-config-tailwindcss), though it may need updates for full v4 support: js { extends: [ 'stylelint-config-recommended', 'stylelint-config-tailwindcss' ] } This prevents Stylelint from flagging Tailwind v4's custom at-rules as unknown. Run stylelint on your CSS files to verify.

Citations:


🌐 Web query:

Tailwind CSS v4 @import "tailwindcss" does it include utilities automatically

💡 Result:

Yes, using @import "tailwindcss"; in Tailwind CSS v4 automatically includes all Tailwind utilities (as well as base styles and theme), replacing the old @tailwind base; @tailwind components; @tailwind utilities; directives from v3. You can then use utility classes like text-xl, bg-blue-500, etc. directly in your HTML.

Citations:


🌐 Web query:

Tailwind CSS v4 @tailwindutilities duplicate if used with@import tailwindcss

💡 Result:

No, using @tailwind utilities alongside @import "tailwindcss" does not cause duplicates in Tailwind CSS v4. The @tailwind directives (base, components, utilities) from v3 have been completely removed in v4. The correct and only way to include Tailwind CSS v4 is with a single @import "tailwindcss"; line at the top of your CSS file. Using any @tailwind directive will result in errors or no styles being generated, as confirmed by official docs and upgrade guide. The perceived "duplicate" issue likely stems from misconfigurations during migration or runtime setups where Tailwind is imported/processed multiple times (e.g., build-time CSS + runtime @import "tailwindcss" source(none); without deduplication), leading to regenerated utilities. This is a user setup problem, not inherent to @tailwind utilities with @import. To fix: - Replace all @tailwind base; @tailwind components; @tailwind utilities; with @import "tailwindcss"; - Use npx @tailwindcss/upgrade for automated migration. - For runtime generation without duplicates, avoid re-importing full Tailwind; use targeted @source or @import "tailwindcss/utilities.css" layer(utilities); with source(none) carefully, as shown in GitHub discussions.

Citations:


Remove @tailwind utilities — it's not valid in Tailwind CSS v4.

In Tailwind CSS v4, @import "tailwindcss" is the only directive you need. The @tailwind directives from v3 (base, components, utilities) have been completely removed. Having @tailwind utilities on line 2 will cause errors or result in no styles being applied.

🧹 Proposed fix
 `@import` "tailwindcss";
-@tailwind utilities;

For the Stylelint false positives, the web search confirms you can suppress them by updating your Stylelint config to recognize Tailwind v4's at-rules. Add an ignoreAtRules array to the at-rule-no-unknown rule (or scss/at-rule-no-unknown for SCSS) with ['theme', 'source', 'apply', 'utility', 'variant', 'plugin', 'config'], or install and extend stylelint-config-tailwindcss if available for your v4 setup.

📝 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
@import "tailwindcss";
@tailwind utilities;
`@import` "tailwindcss";
🧰 Tools
🪛 Stylelint (17.4.0)

[error] 2-2: Unexpected unknown at-rule "@tailwind" (scss/at-rule-no-unknown)

(scss/at-rule-no-unknown)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/styles/app.css` around lines 1 - 2, Remove the obsolete Tailwind v3
directive from your stylesheet by deleting the line containing "@tailwind
utilities" and keep only the "@import \"tailwindcss\"" directive in the
top-level stylesheet (locate the import block in assets/styles/app.css or the
file that contains `@import` "tailwindcss" and `@tailwind` utilities). Also update
your Stylelint config to avoid false positives: modify the at-rule-no-unknown
(or scss/at-rule-no-unknown) rule to include an ignoreAtRules array with
['theme','source','apply','utility','variant','plugin','config'] or
alternatively install/extend a Tailwind v4-aware config such as
stylelint-config-tailwindcss so Tailwind v4 at-rules are recognized.


@theme {
--color-primary: #2563eb;
--color-secondary: #6b7280;
--color-success: #16a34a;
--color-danger: #dc2626;
--color-info: #0891b2;
--color-ext-wf1: #543ff6; /** indigo-500 **/
--color-ext-wf2: #eef2ff; /** indigo-50 **/
--color-ext-wf3: #303F9F; /** indigo-700 **/
}

@source "../../templates/**/*.twig";
@source "../**/*.vue";
29 changes: 8 additions & 21 deletions assets/vue/App.vue
Original file line number Diff line number Diff line change
@@ -1,26 +1,13 @@
<template>
<div>
<h2>Hello from Vue</h2>
<p>{{ message }}</p>
<div class="min-h-screen bg-[#F8FAFC] flex text-slate-900 font-sans">
<AppSidebar />

<div class="flex flex-col flex-1 min-h-screen">
<RouterView />
</div>
</div>
</template>

<script>
export default {
name: 'App',
data() {
return {
message: 'This is a reusable component!'
}
},
created() {
console.log('App component created');
},
mounted() {
console.log('App component mounted');
},
updated() {
console.log('App component updated');
}
}
<script setup>
import AppSidebar from './components/sidebar/AppSidebar.vue'
</script>
58 changes: 58 additions & 0 deletions assets/vue/api.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import {
CampaignClient,
Client,
ListMessagesClient,
ListClient,
StatisticsClient,
SubscribersClient,
SubscriptionClient,
SubscriberAttributesClient,
TemplatesClient
} from '@tatevikgr/rest-api-client';

const appElement = document.getElementById('vue-app');
const apiToken = appElement?.dataset.apiToken;
const apiBaseUrl = appElement?.dataset.apiBaseUrl;

if (!apiBaseUrl) {
console.error('API Base URL is not configured.');
}

const client = new Client(apiBaseUrl || '');
Comment on lines +13 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Proceeding with empty URL after error log can cause confusing failures.

When apiBaseUrl is missing, the code logs an error but then continues with new Client(''). This means API calls will silently fail or hit unexpected endpoints rather than failing fast with a clear message.

Consider throwing or returning early so the app doesn't limp along in a broken state:

🛠️ Suggested fix
 if (!apiBaseUrl) {
     console.error('API Base URL is not configured.');
+    throw new Error('API Base URL is required for the application to function.');
 }

-const client = new Client(apiBaseUrl || '');
+const client = new Client(apiBaseUrl);
📝 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
const appElement = document.getElementById('vue-app');
const apiToken = appElement?.dataset.apiToken;
const apiBaseUrl = appElement?.dataset.apiBaseUrl;
if (!apiBaseUrl) {
console.error('API Base URL is not configured.');
}
const client = new Client(apiBaseUrl || '');
const appElement = document.getElementById('vue-app');
const apiToken = appElement?.dataset.apiToken;
const apiBaseUrl = appElement?.dataset.apiBaseUrl;
if (!apiBaseUrl) {
console.error('API Base URL is not configured.');
throw new Error('API Base URL is required for the application to function.');
}
const client = new Client(apiBaseUrl);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/vue/api.js` around lines 3 - 11, When apiBaseUrl is missing (derived
from appElement?.dataset.apiBaseUrl), do not proceed to construct the Client
with an empty string; instead fail fast by throwing an Error or returning early
so the app won't continue in a broken state. Update the module initialization
around apiBaseUrl, apiToken, appElement and the creation of new Client(...) to
either throw a descriptive Error (e.g., including that apiBaseUrl is required)
or prevent creating Client when apiBaseUrl is falsy, ensuring any calling code
cannot proceed with an empty base URL.


if (apiToken) {
client.setSessionId(apiToken);
}

export const subscribersClient = new SubscribersClient(client);
export const listClient = new ListClient(client);
export const campaignClient = new CampaignClient(client);
export const listMessagesClient = new ListMessagesClient(client);
export const statisticsClient = new StatisticsClient(client);
export const subscriptionClient = new SubscriptionClient(client);
export const subscriberAttributesClient = new SubscriberAttributesClient(client);
export const templateClient = new TemplatesClient(client);

export const fetchAllLists = async ({ limit = 100, maxPages = 100 } = {}) => {
const lists = [];
let afterId = null;

for (let pageIndex = 0; pageIndex < maxPages; pageIndex += 1) {
const response = await listClient.getLists(afterId, limit);
const items = Array.isArray(response?.items) ? response.items : [];
lists.push(...items);

const hasMore = response?.pagination?.hasMore === true;
const nextCursor = response?.pagination?.nextCursor;

if (!hasMore || !Number.isFinite(nextCursor) || nextCursor === afterId) {
break;
}

afterId = nextCursor;
}
Comment on lines +40 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the installed client's getLists signature and pagination shape.
fd -t f 'package.json' node_modules/@tatevikgr/rest-api-client 2>/dev/null | head -5
fd -e d.ts . node_modules/@tatevikgr/rest-api-client 2>/dev/null | xargs -I{} sh -c 'echo "== {} =="; cat "{}"' 2>/dev/null | rg -n -C3 'getLists|nextCursor|pagination'
# Fallback: search repo for any existing usage that hints at cursor type
rg -nP -C2 'nextCursor|getLists\s*\(' --type=js --type=ts --type=vue

Repository: phpList/web-frontend

Length of output: 91


🏁 Script executed:

#!/bin/bash
# Search for nextCursor and getLists usage without problematic rg flags
rg -n -C3 'nextCursor|getLists' assets/ src/ --max-count=20

# Check if the client package is installed and look at its metadata
if [ -d "node_modules/@tatevikgr/rest-api-client" ]; then
  echo "=== Client package found ==="
  ls -la node_modules/@tatevikgr/rest-api-client/ 2>/dev/null | head -20
  find node_modules/@tatevikgr/rest-api-client -name "*.d.ts" -o -name "index.js" | head -10
fi

# Look at the api.js file more broadly to understand initialization
echo "=== Full fetchAllLists context ==="
cat -n assets/vue/api.js | head -60

Repository: phpList/web-frontend

Length of output: 12780


Number.isFinite(nextCursor) breaks pagination with non-numeric cursors.

The API client returns nextCursor as a potentially opaque value (string, UUID, or any truthy type), not a number. Using Number.isFinite() causes this check to fail for string cursors and silently stops pagination even when hasMore is true — you'll get only the first page of results.

The rest of the codebase already uses the correct pattern (e.g., ListSubscribersView.vue:376, CampaignDirectory.vue:779). Relax the check to match:

♻️ Fix
-        if (!hasMore || !Number.isFinite(nextCursor) || nextCursor === afterId) {
+        if (!hasMore || nextCursor == null || nextCursor === afterId) {
             break;
         }

Also consider logging when the loop exits due to maxPages — silent truncation at 10k results could cause confusion later.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/vue/api.js` around lines 40 - 53, The loop stops pagination
incorrectly because Number.isFinite(nextCursor) assumes numeric cursors; replace
that condition with a null/undefined check to allow opaque (string/UUID)
cursors: change the break condition to use nextCursor == null (or nextCursor ===
undefined || nextCursor === null) and keep the existing nextCursor === afterId
check; reference the variables and call: listClient.getLists, nextCursor,
hasMore, afterId. Also add a debug/info log when the loop exits because it
reached the maxPages limit to avoid silent truncation.


return lists;
};

export default client;
33 changes: 33 additions & 0 deletions assets/vue/components/base/BaseBadge.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { mount } from '@vue/test-utils'
import BaseBadge from './BaseBadge.vue'

describe('BaseBadge', () => {
it('renders neutral variant by default', () => {
const wrapper = mount(BaseBadge, {
slots: {
default: 'All',
},
})

const classes = wrapper.get('span').classes()
expect(wrapper.text()).toContain('All')
expect(classes).toContain('bg-gray-100')
expect(classes).toContain('text-gray-800')
})

it('renders counter variant styles', () => {
const wrapper = mount(BaseBadge, {
props: {
variant: 'counter',
},
slots: {
default: '10',
},
})

const classes = wrapper.get('span').classes()
expect(classes).toContain('bg-indigo-50')
expect(classes).toContain('text-ext-wf3')
Comment on lines +28 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify whether `text-ext-wf3` is backed by Tailwind config and find all usages.
# Expected: Tailwind config defines an `ext-wf3` color token, or usages should be changed to a configured token.

set -euo pipefail

echo "== Tailwind config files =="
fd -i '^tailwind\.config\.(js|cjs|mjs|ts)$' -x sh -c '
  printf "\n--- %s ---\n" "$1"
  sed -n "1,240p" "$1"
' sh {}

echo
echo "== text-ext-wf3 / ext-wf3 usages =="
rg -n -C2 '\btext-ext-wf3\b|\bext-wf3\b|\bext_wf3\b' --glob '!node_modules/**' --glob '!vendor/**'

Repository: phpList/web-frontend

Length of output: 9653


Add ext-wf3 to the Tailwind config safelist or extend colors.

The class text-ext-wf3 is used in BaseBadge.vue and other components, but it's not defined in tailwind.config.js—only ext-wf1 is. While the color exists as a CSS variable in app.css, it's missing from the safelist, which means Tailwind might purge it in production builds. Either add the color to theme.extend.colors or add text-ext-wf3 and bg-ext-wf3 to the safelist to ensure it's preserved.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/vue/components/base/BaseBadge.spec.js` around lines 28 - 30, The
failing test uses the utility class text-ext-wf3 that is not preserved by
Tailwind; update tailwind.config.js so this color is never purged by either
adding the color token to theme.extend.colors (e.g., ext-wf3) or adding the
specific utilities 'text-ext-wf3' and 'bg-ext-wf3' to the safelist; modify the
config accordingly so components like BaseBadge.vue (and tests in
BaseBadge.spec.js) can use text-ext-wf3 reliably in production.

expect(wrapper.text()).toContain('10')
})
})
Loading
Loading