[PUI] Notifications are served CUI links

[LavissaWoW]

Please verify that this bug has NOT been raised before.

• [X] I checked and didn't find a similar issue

Describe the bug*

The Notification list API serves link strings that work with CUI. In the case of orders, these do not work, as the URL path no longer uses "order" for all types of orders. So, links to any kind of order do not work.

Steps to Reproduce

1. Trigger a notification on any given order.
2. Attempt to click the supplied notification
3. Experience 404

Expected behaviour

Direct you to the relevant notification.

Deployment Method

• [X] Docker
• [ ] Package
• [ ] Bare metal
• [ ] Other - added info in Steps to Reproduce

Version Information

master

Please verify if you can reproduce this bug on the demo site.

• [ ] I can reproduce this bug on the demo site.

Relevant log output

No response

[LavissaWoW]

I was thinking about using the ModelType enums, but:

{
    "pk": 149,
    "target": {
        "name": "PO0018 - Mouser",
        "model": "Innkjøpsordre",
        "link": "/order/purchase-order/18/"
    },
    "source": {
        "name": "reader",
        "model": "user"
    },
    "user": 2,
    "category": "purchase_order.ready_to_issue",
    "name": "Purchase order is ready to issue",
    "message": "A Purchase order was just marked ready to issue.",
    "creation": "2024-05-26 21:41",
    "age": 4670,
    "age_human": "an hour ago",
    "read": true
},

The model name is translated...

[SchrodingersGat]

I do not think that it makes sense to translate the model name here - does it get displayed in the CUI anywhere?

[LavissaWoW]

/**
 * fills the notification panel when opened
 **/
function openNotificationPanel() {
    var html = '';
    var center_ref = '#notification-center';

    inventreeGet(
        '/api/notifications/',
        {
            read: false,
            ordering: '-creation',
        },
        {
            success: function(response) {
                if (response.length == 0) {
                    html = `<p class='text-muted'><em>{% trans "No unread notifications" %}</em><span class='fas fa-check-circle icon-green float-right'></span></p>`;
                } else {
                    // build up items
                    response.forEach(function(item, index) {
                        html += '<li class="list-group-item">';
                        html += `<div>`;
                        html += `<span class="badge bg-secondary rounded-pill">${item.name}</span>`;
                        html += getReadEditButton(item.pk, item.read, true);
                        html += `</div>`;

                        if (item.target) {
                            var link_text = `${item.target.name}`;
                            if (item.target.link) {
                                link_text = `<a href='${item.target.link}'>${link_text}</a>`;
                            }
                            html += link_text;
                        }

                        html += '<div>';
                        html += `<span class="text-muted"><small>${item.age_human}</small></span>`;
                        html += '</div></li>';
                    });

                    // package up
                    html = `<ul class="list-group">${html}</ul>`;
                }

                // set html
                $(center_ref).html(html);
            }
        }
    );

    $(center_ref).on('click', '.notification-read', function() {
        updateNotificationReadState($(this), true);
    });
}

[LavissaWoW]

I guess the answer is no there

[SchrodingersGat]

Ok, so if we served the model name untranslated, and also included the primary-key of the target, then we could use that to navigate straight to the target link in PUI

[LavissaWoW]

Yup.

Given a valid Modeltype enum value, and a PK, we can create a valid url from the APIEndpoint enum.

Question being: Should we add new ifno, or replace current info? I see the benefit of translating API values, but I'm not sure I feel that those deserve the model variable name.

But I could make a PR that adds a ModelType-respecting model name, and PK to the API endpoint. Then rewrite the PUI code to respect these values rather than the link field.

Lemme know.

[SchrodingersGat]

I'd be happy with a PR which adds new fields, and then we can be certain that the old code is not impacted.

[LavissaWoW]

Would you say ModelType enums generally are model names without _? Like how purchase_order is purchaseorder?

[SchrodingersGat]

Yes, it is the name of the model, just lowercase

[matmair]

This does mean you will need to provide renderes for all model types in PUI.

[SchrodingersGat]

We could potentially pick and choose which ones we wanted to render a link to, and build out only with the ones we support directly in the PUI frontend

[LavissaWoW]

There is already code in place to generate this for PUI. As for the API endpoint itself, that's another story.

In PUI, we simply need to build the URL with the getDetailUrl helper.

I'm drafting a PR for the PUI side of things. The only lacking thing is error report detail pages. It seems we only use a drawer for this, so there's no URL for it atm.

[SchrodingersGat]

It seems we only use a drawer for this, so there's no URL for it atm.

You could just redirect to the error report table, rather than the individual error report

[matmair]

Only a third of all available models have a PUI page in getDetailUrl @LavissaWoW, I have no idea what you are talking about

[LavissaWoW]

For the purposes of notifications, the currently compiled set of models should work well, unless I've missed an outlier or two. The links point to actual webpages, so only the subset of models that have an actual details page even need to be included.

But if you don't like the solution, I'll happily defer this one to you. I'm not really into arguing over these kinds of things.