Skip to content
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

Fix Organisation update functionality #1629

Open
joseAyudarte91 opened this issue Oct 6, 2023 · 0 comments
Open

Fix Organisation update functionality #1629

joseAyudarte91 opened this issue Oct 6, 2023 · 0 comments
Assignees

Comments

@joseAyudarte91
Copy link
Contributor

There are three main issues:

  • We are not handling properly the result of updating the organisation entity in the DB, so we pass wrong reference to the function to associate tags with the target organisation to be updated.
  • We always try to update the 'logo' property (storing 'logo_id'), but some times we do not want to update it, so we would end-up deleting the previous one and not adding any other one replacing it. So we need to update the logo only when it is needed and just delete the previous one if we explicitly get the property as null or we get a new logo to be used.
  • In the front-end side, we get a -1 value for type property when the user selects Other option in the UI, but we need to enforce we get Other string value instead, so the front-end will need to fix this issue, since we are already enforcing this in other endpoints as well to avoid corrupt data.
@joseAyudarte91 joseAyudarte91 self-assigned this Oct 6, 2023
joseAyudarte91 added a commit that referenced this issue Oct 6, 2023
[Re #1629]

We were misusing the result of the DB query to update an organisation,
as we were using the affected rows number as it was the organisation's
updated id, that was being passed to the function to associate tags
with the organisation. Hence, we have ended-up associating those tags
to the organisation with id = `1` (UNEP).

Besides, since we no longer have organisations with id `-1` to handle
GPML membership process, we don't need at all to update an id of an
organisation there, which is quite risky as well. So, now we avoid that
and we also make sure we use the entity's domain properties, removing
the ones that are not expected to be updated in each case.

Finally, we also provide the right `organisation` id coming from the
Path params of the endpoint for making the tags associations.

In order to perform the mentioned changes easier, we have refactored the
update code to be dynamic and not harcode the updated params. For that
we have followed a similar approach as for the resource update query,
adding DB functions declarations as well.
joseAyudarte91 added a commit that referenced this issue Oct 6, 2023
[Re #1629]

We were not handling well logo updates, as we were expecting to always
get it, so if we did not want to update it as part of the rest of the
organisation data, the backend would delete any previous logo image.
Now we make sure we try to delete or replace the logo if we explictly
get this property as part of the update data from the different
endpoints involved.

On the other hand, we have introduced a bug in the previous commit,
since we were expecting a wrong format for the logo coming in the API
requests to update organisation data. We were expecting a URL but that
does not make any sense, since it should be a base64 string, as this is
the content of the logo that is going to be stored. The domain entity's
schema was wrong when it was built and we did not notice that when we
started using it here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant