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

feat: set item's license setting nullable #525

Merged
merged 1 commit into from
May 23, 2024

Conversation

ReidyT
Copy link
Contributor

@ReidyT ReidyT commented May 21, 2024

  • Allows item's license setting to be null.
  • Sending null to the backend will remove the setting.

@ReidyT ReidyT requested review from pyphilia and spaenleh May 21, 2024 14:15
@ReidyT ReidyT self-assigned this May 21, 2024
Copy link
Member

@spaenleh spaenleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

A question, why make a Nullable<T> utility type ?

Also I see that you are removing the "allow" and "alike" variants that were deprecated for the licenses. Is it programmed to have a migration in the backend ? There are currently around 164 instances of items using the "allow" license and 200 using the "alike".

@ReidyT
Copy link
Contributor Author

ReidyT commented May 21, 2024

Thanks for your reply @spaenleh. The Nullable utility, it is primarly to have the possibility to reuse it if we want to allow other settings to be removed.

Concerning the migration, for now I just checked the usage of this deprecated license type in the code and I found no results, so I assumed that it was not used anymore... 😓 But I just saw now that it is still used but with magic value in the UI instead of the SDK type. So I will probably reset my changes and plan a migration in another PR. What do you think ?

@ReidyT ReidyT force-pushed the set-license-item-setting-nullable branch from a0ed410 to 74e924e Compare May 22, 2024 07:56
@ReidyT ReidyT force-pushed the set-license-item-setting-nullable branch from 74e924e to c699dc0 Compare May 23, 2024 11:40
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@ReidyT ReidyT requested a review from spaenleh May 23, 2024 11:40
@ReidyT ReidyT added this pull request to the merge queue May 23, 2024
Merged via the queue into main with commit 2009888 May 23, 2024
3 checks passed
@ReidyT ReidyT deleted the set-license-item-setting-nullable branch May 23, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants