Hi,

can we add the invoice icon to the links on the project page?

Here is a patch for that. In the invoices list all items are already there but only the link on the project page is missing.

Greetings
Carsten

CommentFileSizeAuthor
#15 issue_814820.patch13.96 KBtchurch
#11 issue_814820.patch1.77 KBtchurch
stormproject_invoices.patch1.35 KBcarsten müller

Comments

tchurch’s picture

tchurch commented
Status: Active » Closed (duplicate)

Looking at the patch, I hope it's not that simple.

I would think this is a duplicate of #567558: Extend auto-billing to other types of Storm node

I'm already working on allowing all types to be added to invoices.

Have a look.

juliangb’s picture

juliangb commented
Status: Closed (duplicate) » Needs review

I think we may be talking at cross purposes here.

Perhaps the OP refers to the links which go to automatically filtered lists, rather than the auto-billing?

Status: Needs review » Needs work

The last submitted patch, stormproject_invoices.patch, failed testing.

tchurch’s picture

tchurch commented

Yes. Sorry I see it now. I tested the extra line and I see what it's doing now.

It causes a small hiccup with my changes though.

Can I suggest changing the placement number at the end of each line with $w++

Then other people can add extra links without messing up the number.

See the organization.theme.inc file for example of this.

I'm going to change this in my issue anyway.

carsten müller’s picture

carsten müller commented

Hi,

actually you can add an invoice to a project, right? Or do i misunderstand that. There is a select for organization and for project. So an invoice can directly assigned to a project.
In the list of invoices you can already filter for organization and for projects. So you can select all invoices which are assigned to a project.
But on the project page there is no link to the invoices list filtering for this project nid. The only thing to do is to add a link in the poject links to the invoices adding the nid of the project as filter. Thats all.

URL: storm/invoices?project_nid=xxx

Maybe this case is also handled in the issue #567558: Extend auto-billing to other types of Storm node

Greetings
Carsten

tchurch’s picture

tchurch commented

Yes, I can understand the issue now and can see the benefit too.

If you want me to, I can create a patch file for this and also make the changes in #4 too (this would keep the code for all links sections the same).

Let me know.

carsten müller’s picture

carsten müller commented

Hi,

i don't mind. #6 is just one line. You can add it to your code of #4.

egorovanton’s picture

egorovanton commented
Status: Needs work » Needs review

stormproject_invoices.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, stormproject_invoices.patch, failed testing.

tchurch’s picture

tchurch commented
Assigned: Unassigned » tchurch
Status: Needs work » Active

I will re-work this patch. Probably best to keep it separate, even though it's just one line.
I will also change the other lines in this patch to use $w++.

tchurch’s picture

tchurch commented
Status: Active » Needs review
StatusFileSize
new issue_814820.patch1.77 KB

See how this works.

juliangb’s picture

juliangb commented
Status: Needs review » Needs work

Where is $w defined?

tchurch’s picture

tchurch commented

Honestly I have no idea.

I used the same code structure from stormorganization.theme.inc and stormtask.theme.inc where is does the same thing.

I couldn't see where it was defined there either.

I tested the code and it does work.

juliangb’s picture

juliangb commented

It's minor, but I think the correct thing is to declare - and add it in for the other cases where it hasn't been used.

tchurch’s picture

tchurch commented
Status: Needs work » Needs review
StatusFileSize
new issue_814820.patch13.96 KB

If we're going to declare it here we'd better go through all the theme.inc files and do the same thing there.

I've attached a patch which changes this in all files.

tchurch’s picture

tchurch commented

I can commit this (or try) if people are happy with it.

juliangb’s picture

juliangb commented

Looks good from a read through.

Wonder whether sticking with $w would be neater than $l_pos, but either way this is pretty irrelevant.

tchurch’s picture

tchurch commented
Status: Needs review » Fixed

committed (I hope).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.