Fixes for commerce_node_checkout_expire

Hi
Can you please attach a patch. Only the module maintainers can push.
If you've used git you can do that with
git diff origin/7.x-1.x > /path/to/some/file.patch
Then upload that file.

Thanks
Lee

Just in the process of testing now. Noticed that Relist adds time to the modified date/time not the expiration date.

The relist tab should have the option of being hidden until a selected time. To prevent users from adding time months in advance or in multiples.

I was just looking over the code for the Node expiry, sadly I'm not a programmer so can't fix it myself, but it looks like you just need to create a setting for published and unpublished nodes. Published nodes would create a new expiry date from the current set date, while unpublished nodes would create a brand new expiry date based upon the new checkout time.

PS. Thanks for your hard work. This submodule opens up a ton of new capabilities.

Every time the cron runs, turns the status of the relisted nodes to unpublished. Even if they have been relisted a few minutes before. Is there a quick fix for this?
Thanks for your hard work.

Hi, I wouldn't mind taking a look at the patch. I'm sure others would as well. Thanks :)

Hi drakelam. I think the problem I am facing is related with the #3. Apparently the expiration time is not updated so when the cron runs unpublish the relisted nodes. Can you please tell me what to do for this.

And something else that I've noticed. When there is more than one product option, like, 1 month listing and 1 year listing for the same node, in the relist form you can not see the product names, but two numbers 1 and 11. Look at the attached image.

drakelam can you post a copy of you latest fix/patch/zip file?

removed old message

Any updates on this?

Need to get rid of the info file changes in this patch.

Let's list the issues with this module. I believe I'm going to take a stab at fixing them:

1. The relist form shows numbers instead of actual listing product options.
2. The relist tab shouldn't contain the entire node title (my opinion).
3. Either the rule for reminding users about soon-to-be-expired nodes (Commerce Node Checkout Reminder) should enabled by default or it should become enable when you configure the message.
4. The relist tab should probably say "Renew" if the node hasn't yet expired and the user should be able to add time to prevent it from becoming expired. If the node is expired, it should say "Relist". (or something like that)
5. (related to #4) If you make a first purchase that gives you a node for 1 month, then buy a relisting for another month, the expiration should be set at 2 months from the initial purchase. What you get now is two commerce_line_item entities with expiration dates based on when the purchase was made. This seems like it will be the hardest to fix.

Affects the main module, but should be fixed to enhance this module:

A. The shopping cart items should probably show which node is for each item. People can easily add more than 1 purchase for the same node without knowing.

6. An interface for users to bulk-relist/renew nodes; preferably Views-based. Perhaps either an AJAX form as a Views field to relist, or a VBO action.

7. The query in commerce_node_checkout_expire_load_expiring_line_items() looks like it will unpublish a node based on any of the previous expiration timestamps; whereas it should only be looking at the newest.
8. Looks like it doesn't make any sense for the admin configuration form to be there for the reminder email when it's only the Rule that matters.

9. Looks like the access callback for the Relist page does not check to see if the user owns the node or the product line associated with the node. The only thing it checks is if the node has product lines.
10. The default time-period for the Rule that reminds users of expiring nodes is wrong. It is -10 days but it should be +10 days. -10 days would return nodes that expired 10 days ago. You want nodes that will expire in 10 days.
11. (related to #11) Without tracking whether or not a reminder was sent, the reminder will be sent out on every single cron run.
12. (related to #12) The message should not contain the number of days in which the node expires because the way the query works is it looks for node that will be expired in 10 days. In that lookup, it will also return nodes that will expire in 1, 2, 3, 4, etc, days.
13. The query to lookup expired nodes does not filter for published nodes. You would not want to return nodes that have now become unpublished because Rules would then continue to unpublish unpublished nodes.
14. The query to lookup expired nodes looks up the expiration value based on the field_data_commerce_node_checkout_expires table (via EntityFieldQuery). The problem is that this table can gain an entry even just if there is a node in the cart (that hasn't been bought yet). We need to filter out those entries.
15. The price should be included in the select list label on the relist form.

Okay, here's an epic patch that takes care of almost everything I've listed above (pertaining to this module). I still have a number of things that I think need to be done to the main module (will address those after).

Let me recap my points (referenced above) and what I've done with this patch (started from the module provided above):

1. Fixed. This now shows the products. And the form has been cleaned up.
2. Fixed. This now just says "Relist".
3. Not yet fixed. Unsure about how to properly handle notifications since it seems like we need to track the sending of them otherwise they will continue to be sent out on every cron run over and over again.
4. Unsure what the change should be. I left the title callback for the tab in the module for now. Will have to think about this.
5. Fixed. I made sort of a big change to the way this module handles expiration times. Say you buy a +1 month for a given node. The expires timestamp will be one month from now. Say in a week you buy another month, the way this module works is that you would get another line item entry for +1 month from the given point - but that isn't fair; the time should be added. So what I do now is start the time from the prior expiration (if it's still in the future) and increment off that. When querying for expiration times, we only take the largest timestamp for a given node. Because of this, this module also now enforces that a relisting for a given node can only have 1 item in the shopping cart at a time.
6. Fixed. I added a custom Views field to embed the Relist form inside Views for each node. It is set to use AJAX. I also provided a default View to show it off (disabled). If the given node already has a relisting in the shopping cart, it will provide a link to the cart for that node.
7. Fixed. The query only takes the latest/largest timestamp for the node.
8. Fixed. I removed the admin file since it's not needed.
9. Fixed. I added a check either for the admin permission or that the current user owns the node. Not sure if there is anything else we should do.
10. I changed this but notifications still need work. See #3.
11. See #3.
12. See #3.
13. Fixed. Only published nodes returned.
14. Fixed. I excluded line items in the cart. Not sure if I should just filter for completed checkouts.
15. Fixed.

Minor fix to remove the file inclusion in the menu entry since we are no longer using it.

Let's review this. I think it's more than enough to make this module very usable. The notifications and other small tasks we can open up separate tickets for afterwards.

Going to need to adjust for this newly committed change (#2261333: The product selection widget on nodes should be added via the Field API) then commit and will report back with the new tickets for the remaining issues.

Committed. I will open tickets for the remaining items. Please test and report any new issues in new tickets.