Comments

cha0s’s picture

Sorry, there was a mistake in the last patch... early code that was ditched. Here's the latest.

Island Usurper’s picture

StatusFileSize
new16.03 KB

Oh, wow. This gets my approval big time. Thanks for this.

When I was testing out the forms, I found that the first query in uc_attribute_subject_delete() was confused. You have to do a join there to get the the oid from just the aid and nid/pcid.

I think it's easier to keep all of the API kind of functions together, so I added those new functions after uc_attribute_option_load(). Keeping all of the functions that deal with the database in one place makes them easier for me, at least, to find them when I need to.

cha0s’s picture

Title: Cleanup uc_attribute's product/class API. » Cleanup uc_attribute's product/class API, and add test coverage for the new API, as well as the UI.
StatusFileSize
new27.63 KB
new2.88 KB
new21.21 KB
new19.9 KB

Alright, been cranking on this.

Got 4 patches here... The API functions, implementing the API in the code, a skeleton for product tests, and attribute tests (which need product test structure).

I also put up a branch with the changes as commits instead, for visualizations. (advanced)

Branch at: http://therealcha0s.net/ubercart/branches/488422

Patches should be applied from top to bottom, as they're attached.

Unfortunately, simpletest has some limitations that doesn't allow me to write tests for the class/product attribute UI. (Nested edit elements don't work, e.g. $edit['attributes'][1]['field'])

Island Usurper’s picture

Status: Needs review » Postponed

Gonna put this off until after the 2.0. It's good, but it's too big a change to the API this close to the release.

ezeedub’s picture

I tried applying these patches, but I get the following error on the first one:

$ patch -p0 < 488422.api_.2.x.patch 
patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 456.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej

Should I be applying the earlier ones in this thread first? I'm trying to get Ubercart Devel Generate to work.

Any help is greatly appreciated. Thanks

[EDIT: I tried various combinations of the four, but am not all that familiar with patching, and have since just started over with a fresh ubercart folder.]

cha0s’s picture

Assigned: cha0s » Unassigned

@skriv unfortunately this is what happens when laziness gets the better of a project. The 'maintainers' have had six months to get this (fully tested) patch in and have failed spectacularly. You will find many instances of this within Ubercart's general history. I gave up (a paying job) a while ago due to the immense frustration.

I suggest waiting for the next e-commerce module that will be coming without the word 'Uber' in it. :P

univate’s picture

Yeah it is a shame this has had to sit here this long, this does look like one of the better improvements to ubercart.

splash112’s picture

Sad story

I wouldn't call anybody lazy... But it is definitly a shame that all improvements are off limits till the day that Ubercore sees the light....

univate’s picture

There has got to be a better way... I still don't really have alot of faith of ubercore (the code) being a reality for at least 12-18months or more...

I like what ubercore promises and really want to see it become a reality. I would prefer to see the ubercart team focus on ubercore as goal rather then just starting from scratch:

* when building a new feature there needs to be an API for it (and tests)
* when refactoring something then build an API (and tests)

I think patches like this one would make ubercore a reality alot quicker.

cha0s’s picture

Patches like this are being contributed to another project. :P

splash112’s picture

ayalon’s picture

Sad story... The API would be very helpful.

Andy_Lowe’s picture

Unfortunately, this patch came to late to make it into Ubecart 2.0, and we would be doing a great disservice to developers and user by completely changing the API in a 2.X point release thereby breaking most contrib modules. The results of that would be contrib modules that work for 2.N but not 2.N+1 or vice versa. Very Bad. As a result, we decided to postpone this patch until a major release.

Ubercart 3.0 ( http://drupal.org/node/696816 ) is near completion, and is simply a straight port of Ubercart to Drupal 7. We chose to do a straight port without new features for a few reasons. 1. The Drupal community does not want to make the same mistake it did with Drupal 6 of having a new version that no one can use because critical contrib modules aren't ready. 2. Porting is difficult enough without adding in a bunch of feature / API changes. 3. The vast majority of Ubercart users want a straight port.

As soon as the port is completed, which looks like it will be long before Drupal 7 is ready, we will start adding in features (this one is near the top of the list) for the next release. It is quite possible that early in the D7 life cycle we will have this committed. Committing it any earlier is just not practical. It would result in an unreasonable burden for the developers of Ubercart contributed modules and confused users with broken sites.
--Andy

cha0s’s picture

Your original mistake was the 'straight port' from 5 to 6 without refactoring systems like this that are a nightmare to work with for 3rd party developers.

Your justification for that is by doing it again? Not sure I follow.

univate’s picture

This is worthy of a real review to understand the potential effects of applying it, either to D6 or D7

488422.api_.2.x.patch

* No changes to *.install (ie: no updates to database so will not effect any developers who are interacting with attributes directly via the database)
* Two functions changed:

uc_attribute_load // changed argument order and function implementation
uc_product_get_attributes // removed completely

The change to the first function "uc_attribute_load" is worth a closer look as the order of the arguments have been changed and the implementation of the function also changed which could result in a changed behaviour. One step to ensure compatibility for other developers is the argument order could be changed back. If testing could also show that the implementation did not result in any changed behavior that would also be a good argument.

The second function could be put back to ensure compatibility for any developer using it.

488422.impl_.2.x.patch

This patch changes the other ubercart modules to use attributes to make use of the new API functions rather then interacting directly with the database. Issues with this are that the implementation is different and causes problems.

488422.product_test.2.x.patch
488422.testing.2.x.patch

Addition of simpletests do not effect any functionality.

Issues applying patch to D6

  • The two function changes, if the suggests above are taken into account the effect of this two function changes could be minimised to ensure no actual API are removed only the addition of new API's
  • Implementation differences could result is changed behavior of the attribute system - this could be minimised by not applying the second patch to D6

Issues applying patch to D7

  • API changes should not be an issue as a major version should allow the opportunity to refactor code. The API changes are rather minor as they are mostly about adding an API that is currently non-existent.
  • Implementation is the only issue that presents a potential problem, but with plenty of time to test and the benefit of reducing the SQL in the code, this should also make this a mute point.

Conclusions:
There are definitely issues with committing this to D6 now that there is a stable version, a compromise that might be workable would be committing the API patch and thoroughly testing, while not committing the implementation patch. The D7 issues are all very minor, the argument of wanting to do a straight D6-D7 port doesn't really apply here as this is not about implementing new functionality, it is just about cleaning up the implementation of an existing system and if anything reducing the amount of SQL should be helpful to the port process. Simpletests in D7 should also be encouraged.

univate’s picture

StatusFileSize
new49.1 KB

I've re-rolled this patch so it applies to the current 2.x-dev

This patch includes the API and the two tests patches from #3

I have left out the implementation patch (the 2nd patch) for the reason I have mentioned above - it doesn't add anything for 3rd party developers and there is a risk it could break something. While the API doesn't change much and adds a lot of extra functions for 3rd party devs.

yesct’s picture

tr’s picture

Category: task » feature
Status: Postponed » Needs review

Thanks for the patch univate. You should have reopened the issue!

univate’s picture

I should also note that in the patch above I have made the two changes I suggested in #15

That is:
* changed the argument order in uc_attribute_load so that it doesn't break the existing API
* uc_product_get_attributes - left this function in the code

tr’s picture

@univate: I agree with everything you said in #15. I plan to examine the patch in #16 and test it over the weekend. I'm concerned that the patch/tests might not address checkbox options properly since those were introduced into Ubercart well after #3 was written. The in-memory structure of attributes/options was changed at the same time, and caused a number of problems, especially with contributed modules.

I would appreciate if others would review the patch and test it with a variety of contributed modules. I would like people to try the tests as well - this is a major bonus which will help in getting the patch pushed through.

sethcohn’s picture

+1, seems like the description at http://www.tsd.net.au/blog/understanding-ubercarts-attributes shows why fixing this sooner (D6) rather than later (D7) would be a good thing.

tr’s picture

@sethcohn: Then please test the patch and report back on how it works.

DanGarthwaite’s picture

@sethcohn Funny, that's how I found this page...

torgospizza’s picture

Subscribing with great interest. Are we going to need to re-roll the patch in #16 now that new UC releases are out?

webchick’s picture

This looks like a really nice clean-up. I was bummed to see that save/delete logic was all smooshed into form submit handlers and whatnot. :\

cha0s’s picture

Wow. You guys are definitely earning your keep ;)

Island Usurper’s picture

cha0s, do you have anything useful to say? If not, you can stay out of my issue queue.

Island Usurper’s picture

OK, if no one else is going to test this, I'll say what I've found out.

The patch applies cleanly to the latest version, so it doesn't look like we have to reroll it yet. So far, I've only run the tests, and got two failures: "Attribute integrity check failed." on lines 63 and 100. There are also a bunch of Notices because the site I ran them on was already displaying them. These should be fixed.

I also noticed that uc_product.test doesn't actually have tests in it. If we're going to keep it, we should comment out UbercartProductTestCase::getInfo() so it won't appear on the test list and confuse people.

Is there a reason why implementing this API in Ubercart would cause differences in the way other code would use it? I mean, can it be written in a way that preserves backwards compatibility? Do we even need to strive for that at this point? (For moving to UC 3, we can change that some if we don't run out of time. UC 3 is supposed to come out at the same time as Drupal 7.)

univate’s picture

I have just committed a module in the uc_recurring project that I have been using for some time (6 months+) that relies on this API being available - #569798: Simple Subscriptions UI - so if you are looking to see why this patch is required that is a good example case.

As I mentioned in #15 above the changes in this patch do not change the functionality of ubercart or the attribute system. All it does is puts an API on top of the existing functionality, so us developers don't need to constantly write SQL queues to add remove attributes ourselves. In the patch I re-rolled in #16 I removed all the core changes in ubercart that attempt to use this API, thereby remove most of the risk associated with committing patch.

What "backwards compatibility" issues are you concerned about? as this patch (atleast my re-roll) does not remove any functions, it only add new functions to add the API us developers need to work with attributes.

I will look at the error/warnings you noticed and see if I can't re-roll a patch to fix those.

virtuali1151’s picture

When trying to use the patch in the distrubtion I get these errors.. anybody have ideas? Sorry if this is the wrong thread etc.. I am new to ubercart and just trying to figure out how to apply this patch properly.

Thanks for any assistance in advance.

(Stripping trailing CRs from patch.)
patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej
(Stripping trailing CRs from patch.)
patching file uc_attribute/uc_attribute.test
(Stripping trailing CRs from patch.)
patching file uc_product/uc_product.admin.inc
Hunk #1 FAILED at 389.
Hunk #2 FAILED at 418.
Hunk #3 FAILED at 439.
3 out of 3 hunks FAILED -- saving rejects to file uc_product/uc_product.admin.inc.rej
(Stripping trailing CRs from patch.)
patching file uc_product/uc_product.module
Hunk #1 FAILED at 1948.
1 out of 1 hunk FAILED -- saving rejects to file uc_product/uc_product.module.rej
(Stripping trailing CRs from patch.)
patching file uc_product/uc_product.test

cha0s’s picture

No Lyle, nothing useful, only the patch.

Also, you're blocking this module as well: http://drupal.org/project/uc_generate Devel Ubercart generation.

torgospizza’s picture

@virtuali1151: Are you patching the right version of Ubercart? I think the patch is for 2.4 but I could be wrong.

virtuali1151’s picture

@torgosPizza - Hi Mate... Yes.. I have version 6.x-2.4. Cheers.

torgospizza’s picture

Well if the hunks are failing that probably means the code is not matching up in the diff... looks like the patch will need to be re-rolled. You can usually tell what the issue is by looking at the reject files that patch creates (*.rej). Hope that helps! I haven't had time to patch my system yet otherwise I'd have some feedback myself.

virtuali1151’s picture

StatusFileSize
new6.08 KB

Tks for your help mate. I have attached the .rej files for anybody that wants to take a look and try to fix.

Tks again.

webchick’s picture

#16 applies fine for me. You need to apply it to the latest 6.x-2.x-dev, not 6.x-2.4.

Also, guys, I dunno what sort of personal issue is going on between you, but please keep it off of drupal.org.

virtuali1151’s picture

StatusFileSize
new4.12 KB

@ webchick - Thanks for that info. It patched two of the files correctly but errored on the below:

patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej
patching file uc_attribute/uc_attribute.test
patching file uc_product/uc_product.test

I have attached the .rej for that file. If you have any ideas would be great.

These are the distrubtions I am using.

ubercart dev is: 6.x-2.x-dev
uc recurring: 6.x-2.x-dev

and patch: ubercart-488422-16.patch

univate’s picture

StatusFileSize
new49.22 KB

All tests now pass - issue was the original patch and tests changed the arguments in the function uc_attribute_load, my patch I changed them back to remain 100% backwards compatible with any existing code. Although I do agree that the new argument order does make more sense it should be something for 3.x

I have commented out the UbercartProductTestCase::getInfo() function and made comment to add this back when test are added. This class is required for the attribute tests.

The patch definitely applies cleaning to the current 2.x-dev

virtuali1151’s picture

I just tried this and it failed at the same spot?

virtuali1151’s picture

StatusFileSize
new4.12 KB

Attached is the .rej file that was created.

tr’s picture

Patch in #38 applies cleanly for me.

virtuali1151’s picture

Strange.. I have redownloaded ubertcart version ubercart-6.x-2.x-dev.tar.gz and applied patch (patch -p0 < uc_attribute_api-488422-38.patch) and still getting the same above error? Not sure what I may be doing incorrectly if it working for others?

Any advice is appreciated.

torgospizza’s picture

#42, it looks like the entire patch is getting rejected. Are you sure it's the latest dev? What happens if you rm -rf the sites/all/modules/ubercart dir, then dl the latest dev tarball? (You could also try this on another server that you don't mind deleting an entire module dir).

virtuali1151’s picture

Hi Torgos,

I tried your suggestion and it still errors on the first one. The other two files dont seem to error. I am downloading the latest version from here. (http://drupal.org/project/ubercart/ OR here http://drupal.org/node/280820).

This is the error I am still receiving:

patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej
patching file uc_attribute/uc_attribute.test
patching file uc_product/uc_product.test

arrrgggggggg:)

Thanks again for any advice guys.

virtuali1151’s picture

Can somebody maybe attach their distribution they have used and found it works here? That way I can narrow down the issue abit?

Thanks again.

Cheers.

virtuali1151’s picture

Hi Guys.. Sorry for so many posts here..:) Just trying to figure this out. I have a question maybe you guys can assist with.

1.) If the subscription UI etc is only ready for the old dev version why is it being released in the Alpha distributions? I must apologize for my lack of knowledge here.. I am quite new and just trying to understand the logic as I would love to have this feature enabled for the latest stable version. Any testing etc I can do to get this to work I am more than happy to assist.

Cheers.

Island Usurper’s picture

StatusFileSize
new49.46 KB

And now with all the tests passing all the time, even the ones that had been commented out before. I fixed testAttributeUIAddAttribute() so that the "required" check would work all the time. Remember when using ? : inside a string that it has a lower precedence than .. That means the first part of the string becomes part of the condition, and the next part becomes part of the "else".

univate’s picture

Status: Needs review » Reviewed & tested by the community

I can confirm the test pass and using in a real case with my uc_recurring_subscription module also works as expected.

Island Usurper’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, everyone, for your contributions.

Committed.

virtuali1151’s picture

Hi There,

Would anybody be able to assist with why mine is failing at the following:

Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej
patching file uc_attribute/uc_attribute.test
patching file uc_product/uc_product.test

I will gladly pay for some assistance.

The rej file is attached at #40
Thank you.

torgospizza’s picture

Can you post your uc_attribute.module file?

Also, now that Lyle has committed the patch, you should see this change if you download the newest 6.x-2.x-dev version.

virtuali1151’s picture

StatusFileSize
new17.19 KB

Hi Torgos,

Ok.. I have downloaded the latest version.. and when I enable the Subscription Manager I get the belwo error:

This module requires a patch to ubercart, read the README.txt

Soo then I try this patch like this:

root@svr1 [/home/public_html/sites/all/modules/ubercart]# patch -p0 < 488422_attribute_api.patch
patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej
patching file uc_attribute/uc_attribute.test
patching file uc_product/uc_product.test

and it errors. Attached are the original and rej files.

The only thing I can think of is if the original recurring update changed my DB fields and that is causing it to error? I have no idea if the patch works on the just ubercart code..?? or updates db fields etc..

Again.. thanks for your help mate.

torgospizza’s picture

Hi, the -dev version hasn't updated yet, which is why you're still getting those errors. The latest dev is dated 10/31, you'll probably want the one that will be dated 11/1 or 11/2.

You could also checkout directly from CVS.

virtuali1151’s picture

ahh.. ok.. I will wait..

Tks Torgos.. your a legend.

univate’s picture

Awesome thanks.

Next API issues to knock over now:
#750664: Product Feature API

virtuali1151’s picture

Hi Guys,

Ok.. the latest distribution works without errors except when I set the Subscription Settings product class to membership type... instead of the recurring subscription.... not sure if this is because I already have some memberships etc defined or not..??

•warning: Invalid argument supplied for foreach() in /home/public_html/sites/all/modules/uc_recurring/modules/uc_recurring_subscription/uc_recurring_subscription.admin.inc on line 46.

Thanks again guys.

Cheers.

virtuali1151’s picture

Hi guys,

Anybody have an idea why this error is happening?

•warning: Invalid argument supplied for foreach() in /home/public_html/sites/all/modules/uc_recurring/modules/uc_recurring_subscription/uc_recurring_subscription.admin.inc on line 46.

rszrama’s picture

Just so we're clear on what to expect when we upgrade, this patch seems to just add an attribute API with test coverage. It does not seem to be changing any data, so any custom code we have interacting with attributes should still work. Does this hold true for data returned by the uc_attribute_load() function, i.e. that we can expect it to return the same data it used to?

I also don't see this patch updating any attribute related forms to actually use the API. Did I miss something there? Is there another patch handling that?

univate’s picture

Hi Ryan, thats correct.

There is a patch in #3 that also changed ubercart's implementation to use these new api's, but as I stated in #15 that part of this issue added the most risk and so my suggestion was to leave out that part of the patch, which it was, so ubercart still works exactly the same as before this patch (no functions have been removed and the database schema is still exactly the same).

The goal here was just to provide functions module developers need to leverage the attribute system without having to mess with the database directly.

rszrama’s picture

Good to know. We have a couple really custom implementations of Attributes and I wanted to make sure we'd be able to safely update. It sounds like your approach covers our tail. : )

sorensong’s picture

Ok, so if I'm using Subscription Manager 6.x-2.0-alpha5, which patch do i need to apply?

lunk rat’s picture

Same question: which patch to patch uc core for Subscription Manager 6.x-2.0-alpha5?

I ran the patch that comes with the module but I get the error from #57 as well as the following when I view the product:

warning: Invalid argument supplied for foreach() in /home/public_html/sites/all/modules/ubercart/uc_attribute/uc_attribute.module on line 1174.

New to recurring stuff

univate’s picture

This issue is fixed and has nothing to do with uc_recurring - if you want to discuss uc_recurring related issue, you should do so in the uc_recurring issue queue.

The specific issue discussing the subscription manager/ui module can be found at:
#569798: Simple Subscriptions UI

virtuali1151’s picture

Hi All,

The below error is still happening? Should we post this in the recurring que..??

•warning: Invalid argument supplied for foreach() in /home/public_html/sites/all/modules/uc_recurring/modules/uc_recurring_subscription/uc_recurring_subscription.admin.inc on line 46.

If your current setup is that you have recurring subscriptions as your product class type.. then you change it to membership type in the subscription settings is when this is triggered. It gives the above error on all current subscriptions.

Cheers.

torgospizza’s picture

virtual, yes. That is a uc_recurring issue, and this is the queue for Ubercart.

virtuali1151’s picture

Ok.. no worries.. I added it over there...

Thanks mate.

kasyap5’s picture

StatusFileSize
new31.11 KB

Hi All,

I am new to drupal .. and I am getting the following error after i install recurring subscription module :

warning: Invalid argument supplied for foreach() in /home/ey9oguhf/public_html/vm.hktechsol.com/modules/ubercart/uc_attribute/uc_attribute.module on line 1174.

can u let me know which patch i need to apply for uc_attribute module?

I am using drupal 6.19

Thanks in advance ..

Mixologic’s picture

@kasyap5 - Typically the best way to find help with an error that you are having is to post a support request or bug report in the issue queue of the module that is causing you the error, even though uc_recurring is part of Ubercart, it is its own distinct module with its own maintainers that can give you the best advice. (Even though the maintainers of that module also happen to be on this thread).. But in general your best bet is to post to the http://drupal.org/project/issues/uc_recurring issue queue for your particular error, since you said it started when you enabled uc_recurring. This issue also happens to be marked fixed, which means people are less likely to be reading it at all. But in general, if your problem isnt directly related to the issue topic, its better to open another topic. You'll notice that Lunk Rat posted the same error you have in comment 62, and was also directed to the uc_recurring issue queue, but nobody has posted an issue there so its possible nobody knows about it.

Best of luck with your issue.

kasyap5’s picture

Thanks mixologic ... the reason why i came to this page is that i got instructions in README file of the recurring subscription package as follows:


uc_recurring_subscription
~~~~~~~~~~~~~~~~~~~~~~~~~

uc_recurring_subscription is a drupal module that integrates recurring payments
and roles and provides a set of features specifically designed for managing a
membership/subscription website.

INSTALL
~~~~~~~
This module requires a patch to ubercart, more detail on the issue can be found
in the following issue:
http://drupal.org/node/488422

The patch required for this module is included in this project download under:
module/uc_recurring_subscription/ubercart_api.patch

To apply:
1. Copy the ubercart_api.patch file to ubercart module directory.
2. Patch by running the command:
patch -p0 < ubercart_api.patch
3. You shouldn't get any errors if it applies correctly.

so i wonder where would i get the correct "ubercart_api.patch" file ..

torgospizza’s picture

I think the patch has been committed, which is why this issue is "fixed". If that's the case, then the documentation you quoted should be updated as well.

ronvanherk’s picture

Category: feature » support

Hi,

I have installed the UC Subscriptions module, but Drupal keeps telling me to patch ubercart.

I tried about all the patches in this topic (and the others) but I just keep getting errors like:
root@CoastecTV:/var/www/modules# patch -p0 < ubercart-488422-16.patch
patching file uc_attribute/uc_attribute.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_attribute/uc_attribute.module.rej

My Ubercart module is up to date: 6.x-2.4, and UC Recurring Payments and Subscriptions is 6.x-2.0-alpha5

Any help would be highly appreciated, as I can't use the module right now. After adding a subscription, the screen goes white and nothing else happens..

Best regards,
Ron

tr’s picture

Status: Fixed » Closed (fixed)

You need to read the entire issue. The patch that got committed is in #47. Closing this issue - this is not the right place to keep discussing problems with uc_recurring.

summit’s picture

Hi,

Is with this patch (#47) this explained problem solved: http://www.tsd.net.au/blog/understanding-ubercarts-attributes
I just want to make sure what the system is doing when I go from Ubercart 2.4 to .dev?

Thanks a lot in advance for your reply still on this closed issue.

greetings,
Martijn

univate’s picture

@summit - what problem are you trying to solve?

This patch does not make any functional changes, all it does is add an API.

summit’s picture

Hi, I would like to have Ubercart working nicely with attributes as explained in: http://www.tsd.net.au/blog/understanding-ubercarts-attributes?
Greetings, Martijn

torgospizza’s picture

Is there something specific in that page you're wondering about?

summit’s picture

The 4 thingies, because there is referred to this page, I thought I ask it here.

   1. Form a User Interface (UI) perspective long and tedious. There is no interface for creating attribute options on mass. 
   2. Changes to the Product Class attribute/options doesn't reflext in existing instances of that class.
   3. When new Attribute Options are added then the uc_product_adjustments table has to be flushed, losing all user input and recreated again (well it doesn't have to be but thats the laziest way to code it else you'd have to pull all the records out so you could unserialise the options just to see whats already there, one of the many disadvantages to not Normalising your DB schema.
   4. Mostly this not consistent with (A) in that the realisation that the SKU is the real Product identifier in that each SKU represents a model in the stote room, the physical product. The Product with attributes is an illustion presented to the customer to make navigating the long list of every product combination easier to understand. Its conceptual not actual. In order to keep with (A) one would have to then create a Node for each Attribute Option combination. This coul be an option but then you'd have the replication of standard Node data like Title & Description for each, which would be a maintenance problem trying to keep them all in sync (and then translated!).

greetings, Martijn

torgospizza’s picture

I see. These are still issues, because as as univate explained in #74, this patch does not change any functionality, it just adds some new API methods that we can use to (eventually) address the shortcomings as mentioned on that page.

summit’s picture

After more investigation I saw some threads about these thingies, shall I open another issue with them? Or is this the right place still?
greetings, Martijn

torgospizza’s picture

I'd create a new Issue.

summit’s picture

Ok filed a discussion issue on http://drupal.org/node/1010986. I really like the way Ubercart works, but also that all sorts of people are working on still improving the drupal 6 version!.
greetings, Martijn

BeaPower’s picture

I tried patch #47 for alpha version and I get this:

File to patch: uc_attribute/uc_attribute.module'
uc_attribute/uc_attribute.module': No such file or directory
Skip this patch? [y] n
File to patch: uc_recurring.module
patching file uc_recurring.module
Hunk #1 FAILED at 538.
1 out of 1 hunk FAILED -- saving rejects to file uc_recurring.module.rej
patching file uc_attribute/uc_attribute.test
patching file uc_product/uc_product.test

Did it work? If not, where is the working patch?

univate’s picture

This issue is now CLOSED!!! the patch is already in ubercart's current 6.x-2.x-dev.

shensman’s picture

When you say the issue is CLOSED, do you mean we don't need any patch anymore?

I have installed 6.x-2.x-dev dated on January 14, 2011, but still get the following message:

This module requires a patch to ubercart, read the README.txt

torgospizza’s picture

Yes, that's what that means. Sounds like that message is still in there by mistake - probably an oversight.

shensman’s picture

So hopefully the mistake will be fixed very soonn?

emilianodelau’s picture

Status: Closed (fixed) » Needs review

I get the following fatal error when trying to install the module.

Fatal error: Call to undefined function uc_attribute_save() in /myHomeDirectory/sites/all/modules/uc_recurring/modules/uc_recurring_subscription/uc_recurring_subscription.install on line 115

tr’s picture

Status: Needs review » Closed (fixed)
bdsweb’s picture

Hello... I'm using the latest n greatest dev (May 11, 2011) and am still seeing the errors everyone has reported here.

warning: Invalid argument supplied for foreach() in /sites/all/modules/ubercart/uc_attribute/uc_attribute.module on line 1174

in #84 & #85 you say we don't need the patch, that it's rolled into the latest. Could someone help with this?

Thanks!

bdsweb’s picture

Status: Closed (fixed) » Active

Plus there are Attributes that can't be removed... I go in to edit a product's attributes, click the far left option to remove, click "save changes" and the attribute is still there. It's now blank and says "n/a" below it. 4 are listed on the product page, but only 3 listed in the db.

Thanks for looking into this

bdsweb’s picture

Also, when I try to rearrange the order of attributes on the Edit Attributes page, it deletes the attribute name and locks me from adding it back in... I essentially have to cancel the edit I was trying to do in the first place.

bdsweb’s picture

Status: Active » Closed (fixed)

Ok, don't know why, but if I delete all my products and re-enter all of them all over again (including resetting the attributes) it appears to work without errors. Something in the update isn't retro-active for info already in the db.

Closed... but a very frustrating problem.

kevinpattison’s picture

StatusFileSize
new6.94 KB

Hey,

I'm trying to patch a fresh install of ubercart and I'm getting:

patching file uc_attribute/uc_attribute.admin.inc
Hunk #1 succeeded at 529 (offset 30 lines).
Hunk #2 FAILED at 627.
Hunk #3 succeeded at 825 (offset 50 lines).
Hunk #4 succeeded at 833 (offset 50 lines).
Hunk #5 succeeded at 875 (offset 51 lines).
Hunk #6 succeeded at 897 (offset 51 lines).
Hunk #7 succeeded at 978 (offset 91 lines).
Hunk #8 FAILED at 899.
2 out of 8 hunks FAILED -- saving rejects to file uc_attribute/uc_attribute.admin.inc.rej
patching file uc_attribute/uc_attribute.module
Hunk #1 succeeded at 339 (offset 47 lines).
Hunk #2 succeeded at 362 (offset 47 lines).
Hunk #3 succeeded at 894 (offset 107 lines).

Rejection file attached. How can
I fix this?