Profile Not deleting Bug #3526

heintayzar-hm · 2023-04-03T18:02:10Z

Resolves #3428

Description

Type of change

Bug fix (non-breaking change which fixes an issue)
Workaround

In this Pr, I did the following:

When partners update their profile to an empty value, we will also change it into an "N/A" value(as said in the issue).

When the organization updates its profile to an empty value, we will also change it into an "N/A" value(as said in the issue).

How Has This Been Tested?

In this pr, I test the above functionalities with empty values.

…troller

dorner

Hey @heintayzar-hm thanks for the PR! I think there should be a way to solve this that doesn't involve putting sentinel strings into the database.

dorner · 2023-04-03T20:55:15Z

app/controllers/partners/profiles_controller.rb

@@ -90,7 +90,7 @@ def profile_params
        :enable_individual_requests,
        :enable_quantity_based_requests,
        documents: []
-      ).select { |_, v| v.present? }
+      )


There was a reason this had to happen before... there was some error that was happening. I don't remember why. 😢

@dorner , Yeah when I removed that I actually got the error from: #3446

reason that I removed

So the thing here is we check with v.present?. So if there is no value, we are not getting any values from params and not updating and causing this bug.

possible error that you metioned

As for the error that you talked I think it is this one(but I am not very sure):
#3446

if we don't include v.present it is updating files(previously) and getting the error.

I don't know whether I keep the current change or (I can compare the params data and database profile data and then update but this is the current way the only thing that changed is the place we filter things). Please let me know about this one, thank you/

Maybe what we can do is instead of checking if the value is present, we check if the key is present in the parameters. That should allow us to save blank values. If that still doesn't work, maybe we handle file uploads separately.

dorner · 2023-04-03T20:55:44Z

app/models/partners/profile.rb

@@ -106,7 +106,7 @@ class Profile < Base
    ]

    def has_no_social_media?
-      website.blank? && twitter.blank? && facebook.blank? && instagram.blank?
+      (website.blank? || website == "N/A") && (twitter.blank? || twitter == "N/A") && (facebook.blank? || facebook == "N/A") && (instagram.blank? || instagram == "N/A")


We shouldn't be forced to use "N/A" when an empty string also indicates that there is no value.

Ok I will work on that

heintayzar-hm · 2023-04-07T16:59:32Z

Hi, it is done. But for showing "N/A" on the profile. I have done it without storing the string in the database but it is affecting other test cases. So I didn't push on this branch.
This is the PR for Updating "N/A" #3532

dorner · 2023-04-14T19:45:58Z

@heintayzar-hm I'm confused about how these 2 PRs are related. It seems like both of them fix the same problem, but this one is "incomplete" because it does it by using "N/A", which we said we didn't want to use?

Can we combine these two into a single solution?

heintayzar-hm · 2023-04-14T20:47:50Z

@dorner ok, the reason I divided two prs is in this pr #3532, the changes that I made is affecting other test cases as I changed model getter methods. in this #3532, so I am making sure to include in this pr or not, by your final message, so should I discard the #3532 and did not include "N/A" part?

dorner · 2023-04-18T02:37:46Z

@heintayzar-hm I'm not sure I fully understand your question. All I'm really looking for is something that fixes this issue without the additional baggage of an "N/A" sentinel value. If you have a solution for that, then it should be part of a single PR rather than split into two.

heintayzar-hm · 2023-04-18T12:44:38Z

@dorner , it is working and please review again this pr, previous there is a misunderstanding in my end , I am sorry.

But Make sure to reset the database and seed it back cause there might be previous update,

if there is a problem, please let me know 😄😄

dorner · 2023-04-19T00:46:01Z

@heintayzar-hm sorry, I'm still confused. The current PR is still setting values to "N/A" instead of blanks.

heintayzar-hm · 2023-04-19T07:20:05Z

Just to be sure, what you want is not setting anything, inside the input fields, not even the value "N/A". (The current pr does not store anything inside the database, I really thought you don't want to only add "N/A", in the database), So if it is tell me, because I think our communication was off 😐😐.

dorner · 2023-04-21T19:13:57Z

Correct. When you save a blank field, it should set it to a blank string and that's what should show up on the front-end.

heintayzar-hm · 2023-04-25T11:19:14Z

In these changes, I removed the N/A showing in the frontend. @dorner

dorner · 2023-04-27T00:41:59Z

spec/requests/partners/profiles_requests_spec.rb

+      end
+
+      it "does not store N/A in the database" do
+        expect(partner.profile.website).to be_nil


Wait, is there something that deliberately changes the value "N/A" to nil? If this test passes, I'm not sure why.

Not really, It was because I forgot to add reload, it should be partner.profile.reload.website as I a use put request in before block. The caching is causing the problem. I will update with the changes.

heintayzar-hm · 2023-04-27T08:11:45Z

Hi, I finished the testing part, but if you look at my latest commits(latest fours commits), I found that the schema.rb file is updating differently on my side(since I updated to ubuntu 23.04), SO it is changing automatically, in case it will mess up others code, I change it back to previous one.

dorner · 2023-04-27T13:50:36Z

@heintayzar-hm this looks good!

heintayzar-hm added 4 commits April 3, 2023 23:10

Fix the problem with partner profile blank not updating

1758220

Merge branch 'main' into profile-error

5c0724e

Fix the validation problems due to adding N\a

99ca24f

Fix failed test cases and Fix partner profile the same as partner con…

ef526e8

…troller

heintayzar-hm changed the title ~~Profile error~~ Profile Not deleting Bug Apr 3, 2023

dorner requested changes Apr 3, 2023

View reviewed changes

heintayzar-hm added 4 commits April 7, 2023 19:50

Handle sentital data storing in db

e4fca94

Merge branch 'main' into profile-error

e1b717e

Work arounds

ba0cced

Fix profile error

521d4d7

Add for profile testing

e13cdaa

heintayzar-hm requested a review from dorner April 7, 2023 17:15

heintayzar-hm mentioned this pull request Apr 9, 2023

[BUG] Partner profile -- fields are not saving if blanked #3428

Closed

3 tasks

Combine two prs

0cc9931

heintayzar-hm mentioned this pull request Apr 18, 2023

Profile testing error #3532

Closed

heintayzar-hm added 3 commits April 18, 2023 18:41

Finish tessting

8b6c40e

CHnage the path

3004626

Fix Linters

986894d

Finish the removing of NA showing in frontend

7901a46

dorner requested changes Apr 27, 2023

View reviewed changes

FInish test error

73ae721

heintayzar-hm added 3 commits April 27, 2023 14:13

Test the test cases

27de1e0

Change back the version

ff089c3

Change back the version

312901e

heintayzar-hm added 2 commits April 27, 2023 15:00

Merge branch 'main' into profile-error

2408568

Change back the version

271fb66

dorner approved these changes Apr 27, 2023

View reviewed changes

Merge branch 'main' into profile-error

62ef68c

dorner merged commit 39fdaa7 into rubyforgood:main Apr 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profile Not deleting Bug #3526

Profile Not deleting Bug #3526

heintayzar-hm commented Apr 3, 2023 •

edited

Loading

dorner left a comment

dorner Apr 3, 2023

heintayzar-hm Apr 3, 2023 •

edited

Loading

dorner Apr 4, 2023 •

edited

Loading

dorner Apr 3, 2023

heintayzar-hm Apr 3, 2023

heintayzar-hm commented Apr 7, 2023 •

edited

Loading

dorner commented Apr 14, 2023

heintayzar-hm commented Apr 14, 2023

dorner commented Apr 18, 2023

heintayzar-hm commented Apr 18, 2023

dorner commented Apr 19, 2023

heintayzar-hm commented Apr 19, 2023

dorner commented Apr 21, 2023

heintayzar-hm commented Apr 25, 2023

dorner Apr 27, 2023

heintayzar-hm Apr 27, 2023

heintayzar-hm commented Apr 27, 2023

dorner commented Apr 27, 2023

Profile Not deleting Bug #3526

Profile Not deleting Bug #3526

Conversation

heintayzar-hm commented Apr 3, 2023 • edited Loading

Description

Type of change

How Has This Been Tested?

dorner left a comment

Choose a reason for hiding this comment

dorner Apr 3, 2023

Choose a reason for hiding this comment

heintayzar-hm Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

reason that I removed

possible error that you metioned

dorner Apr 4, 2023 • edited Loading

Choose a reason for hiding this comment

dorner Apr 3, 2023

Choose a reason for hiding this comment

heintayzar-hm Apr 3, 2023

Choose a reason for hiding this comment

heintayzar-hm commented Apr 7, 2023 • edited Loading

dorner commented Apr 14, 2023

heintayzar-hm commented Apr 14, 2023

dorner commented Apr 18, 2023

heintayzar-hm commented Apr 18, 2023

But Make sure to reset the database and seed it back cause there might be previous update,

dorner commented Apr 19, 2023

heintayzar-hm commented Apr 19, 2023

dorner commented Apr 21, 2023

heintayzar-hm commented Apr 25, 2023

dorner Apr 27, 2023

Choose a reason for hiding this comment

heintayzar-hm Apr 27, 2023

Choose a reason for hiding this comment

heintayzar-hm commented Apr 27, 2023

dorner commented Apr 27, 2023

heintayzar-hm commented Apr 3, 2023 •

edited

Loading

heintayzar-hm Apr 3, 2023 •

edited

Loading

dorner Apr 4, 2023 •

edited

Loading

heintayzar-hm commented Apr 7, 2023 •

edited

Loading