Add read-only inventory permission (view stock & selling price only) by henri1024 · Pull Request #4358 · opensourcepos/opensourcepos

henri1024 · 2025-12-24T18:23:20Z

🧾 Description

This PR introduces a new read-only inventory permission that allows specific users (e.g. shop intern) to:

View current stock quantity
View selling price

while restricting access to:

Product cost / purchase price
Inventory creation, update, or deletion
Any inventory-related configuration

This feature is intended for retail shops where clerks should be able to sell items and check stock availability without being able to modify inventory data or see sensitive cost information.

✨ Key Changes

Added a new permission role: items_view
Inventory module now checks permission before:
- Displaying product cost fields
- Allowing create / edit / delete actions
UI hides cost-related columns when permission is not granted
Selling price and stock remain visible for authorized users
Backend validation added to prevent unauthorized access via direct URL or API calls

🔐 Permission Behavior

Action	Admin	Shop Clerk (view_inventory_basic)
View stock	✅	✅
View selling price	✅	✅
View product cost	✅	❌
Add / edit product	✅	❌
Delete product	✅	❌

🎯 Use Case

Small to medium retail shops
Clear separation between owner/admin and shop clerk roles
Prevents accidental or intentional inventory changes
Protects sensitive cost and margin information

🧪 Testing

Tested with multiple user roles
Verified UI behavior and direct URL access
Confirmed product cost is not exposed in inventory views or reports

📎 Notes

Backward compatible
Existing installations are unaffected unless the new permission is explicitly assigned

…management

objecttothis

Take a look at my comments and let me know what you think.

objecttothis · 2025-12-28T13:12:13Z

app/Controllers/Items.php

+    /**
+     * Check if user has items_stock permission
+     */
+    private function has_stock_permission(): bool


These functions seem like they could be used by other controllers. Maybe it is better to place them into something like Security_helper.php or a common place so that it can be used globally.

objecttothis · 2025-12-28T18:16:50Z

app/Controllers/Items.php

     */
    public function getInventory(int $item_id = NEW_ENTRY): void
    {
+        // Check permission: only users with stock permission can update inventory


This comment probably isn't necessary. The if statement makes it pretty clear what the code is doing.

objecttothis · 2025-12-28T18:17:06Z

app/Controllers/Items.php

     */
    public function postSave(int $item_id = NEW_ENTRY): void
    {
+        // Check if user has items_stock permission


objecttothis · 2025-12-28T18:17:32Z

app/Controllers/Items.php

     */
    public function postSaveInventory($item_id = NEW_ENTRY): void
    {
+        // Check permission: only users with stock permission can update inventory


Same here and below.

objecttothis · 2025-12-28T18:18:49Z

app/Database/Migrations/20251225000000_add_items_view_permission.php

+
+use CodeIgniter\Database\Migration;
+
+class Migration_add_items_view_permission extends Migration


When we enable the linter, it's not going to like this class naming.

objecttothis · 2025-12-28T18:22:58Z

app/Database/tables.sql


 INSERT INTO `ospos_permissions` (`permission_id`, `module_id`, `location_id`) VALUES
    ('items_stock', 'items', 1),
+    ('items_view', 'items', 1),


I think this file is frozen and this needs to be done through a migration.

correct indeed, migration alone is sufficient

objecttothis · 2025-12-28T18:25:19Z

app/Helpers/tabular_helper.php

    $item_taxes = model(Item_taxes::class);
    $tax_category = model(Tax_category::class);
    $config = config(OSPOS::class)->settings;
+    $has_stock_permission = $permissions['has_stock_permission'] ?? true;


This probably should use PSR-12 compliant naming so that we are constantly moving toward PSR-12 compliance rather than away from it. $hasStockPermission.

objecttothis · 2025-12-28T18:26:32Z

app/Helpers/tabular_helper.php

+    $icons = [];
+
+    // Only show edit button if user has stock permission
+    if ($has_stock_permission) {


Couldn't all three of these be placed under just one if ($has_stock_permission) { check?

objecttothis · 2025-12-28T18:32:42Z

app/Models/Stock_location.php

+     * @param string $location_name
+     * @return void
+     */
+    private function _insert_new_subpermission(string $permission_id, int $location_id, string $location_name): void


Since PHP has function scoping (since PHP 5.0), hungarian notation to notate private functions is obsolete. I think this is why we added the TODO on line 189. I don't think we should add new functions with hungarian notation, but instead move to a PSR compliant naming for the function and for parameters. private function insertNewSubpermission(string $permissionId, int $locationId, string $locationName): void

objecttothis · 2025-12-28T18:33:32Z

app/Views/items/manage.php

 use App\Models\Employee;
+
+$employee = model(Employee::class);
+$logged_in_employee = $employee->get_logged_in_employee_info();


let's make these variable names PSR compliant.

jekkos · 2025-12-30T22:58:53Z

app/Database/Migrations/20251225000000_add_items_view_permission.php

+            $this->db->table('ospos_permissions')->insert($permission_data);
+            log_message('info', 'Inserted items_view permission.');
+
+            // Grant items_view permission to all existing employees


@objecttothis should we grant this permission to an employee if she does not have access to the items module? I would also add this condition.

jekkos · 2025-12-30T23:00:33Z

app/Database/Migrations/20251225000000_add_items_view_permission.php

+            $permission_data = [
+                'permission_id' => 'items_view',
+                'module_id'     => 'items',
+                'location_id'   => 1


we should insert this data for all current active location ids. Please check if we can get rid of the hardcoded id here by iterating over active stock locations for an employee and generate the data for each of them.

jekkos · 2025-12-30T23:02:00Z

app/Controllers/Items.php


+        // Mask cost_price for view-only users
+        if (!$this->has_stock_permission()) {
+            $item_info->cost_price = 0;


I hope this can't lead to accidental cost price updates ? I guess if this condition holds then the user will not be able to post back the values?

jekkos · 2025-12-30T23:02:30Z

Thanks for this useful addition, hope you can get the comments worked out so we can merge it.

Add items_view permission and update related functionality for stock …

90a744a

…management

jekkos requested a review from objecttothis December 27, 2025 14:11

jekkos self-assigned this Dec 27, 2025

objecttothis requested changes Dec 28, 2025

View reviewed changes

jekkos reviewed Dec 30, 2025

View reviewed changes


		use CodeIgniter\Database\Migration;

		class Migration_add_items_view_permission extends Migration

Uh oh!

Conversation

henri1024 commented Dec 24, 2025

🧾 Description

✨ Key Changes

🔐 Permission Behavior

🎯 Use Case

🧪 Testing

📎 Notes

Uh oh!

objecttothis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jekkos commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants