-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jw debugging cart remove add actions #730
Changes from 5 commits
7f431ee
4022e05
5a0f0e9
1ca2919
ad88d11
b44db9c
4c81626
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -259,11 +259,9 @@ def add_service | |
# create sub_service_requests | ||
@service_request.reload | ||
@service_request.previous_submitted_at = @service_request.submitted_at | ||
|
||
new_line_items.each do |li| | ||
ssr = find_or_create_sub_service_request(li, @service_request) | ||
li.update_attribute(:sub_service_request_id, ssr.id) | ||
|
||
if @service_request.status == 'first_draft' | ||
ssr.update_attribute(:status, 'first_draft') | ||
elsif ssr.status.nil? || (ssr.can_be_edited? && ssr_has_changed?(@service_request, ssr) && (ssr.status != 'complete')) | ||
|
@@ -327,9 +325,12 @@ def remove_service | |
end | ||
|
||
@service_request.reload | ||
|
||
@line_items_count = @sub_service_request ? @sub_service_request.line_items.count : @service_request.line_items.count | ||
@sub_service_requests = @service_request.cart_sub_service_requests | ||
|
||
respond_to do |format| | ||
format.js {render layout: false} | ||
end | ||
end | ||
|
||
def get_help | ||
|
@@ -571,7 +572,7 @@ def send_individual_service_provider_notification(service_request, sub_service_r | |
line_items = [] | ||
@service_request.sub_service_requests.each do |ssr| | ||
if service_provider.identity.is_service_provider?(ssr) | ||
line_items << SubServiceRequest.find(ssr).line_items | ||
line_items << SubServiceRequest.find(ssr.id).line_items | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we finding the SubServiceRequest when we already have it from the .each iteration? Can't we do line_items << ssr.line_items? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kyle-glick I was fixing a deprecation warning here, but yes! Good catch. |
||
end | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,4 +17,4 @@ | |
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS | ||
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR | ||
# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
$('.cart').replaceWith("<%= escape_javascript(render( 'service_requests/right_navigation/cart', service_request: @service_request, sub_service_request: @sub_service_request, sub_service_requests: @sub_service_requests, line_items_count: @line_items_count, allow_delete: true )) %>") | ||
$('.shopping-cart').html("<%= escape_javascript(render( 'service_requests/right_navigation/cart', service_request: @service_request, sub_service_request: @sub_service_request, sub_service_requests: @sub_service_requests, line_items_count: @line_items_count, allow_delete: true )) %>") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 - I think html() is the right choice here (http://stackoverflow.com/questions/730916/whats-the-difference-between-jquerys-replacewith-and-html) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a local handler? I noticed that the problem with the original code was that as you would remove/add more services, the handlers would add up and multiple clicks would be registered when you click the 'Yes' button. @jwiel86 Can you confirm?
If this does work, we should remove the two above lines setting data attributes on the yes button because they won't be needed anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kyle-glick When I was debugging this file, the variables set after $(document).on 'click', '.remove-service', -> were not being reset. So if you deleted one line_item, everything would work fine. However, if you deleted a second one. It would try to execute the code with the previous line item id and then it would also execute the code for the line item you just removed (I think that is what you were describing above- multiple clicks). Changing it to this selector on click format solved the problem for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this onClick handler can only run once. Whole modal + .yes-button re-rendered on line 41.