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

Adding copyright to albums #1838 #1880

Merged
merged 17 commits into from
Apr 28, 2024
Merged

Conversation

ThanasisMpalatsoukas
Copy link
Contributor

@ThanasisMpalatsoukas ThanasisMpalatsoukas commented Jun 17, 2023

Fixes #1838

Bringing the data of copyright to the frontend and adding a new route on api.php to edit the copyright value.
for issue #1838. After getting this PR accepted i will work on the frontend side and push on frontend the rest

Bringing the data of copyright to the frontend and adding a new route
on api.php to edit the copyright value.
@ildyria
Copy link
Member

ildyria commented Jun 17, 2023

Thank you for your pull request. :)

Could you please add two tests:

  • one where someone adds a copyright
  • one where someone removes the copyright.

Also what should be the default value when it is not present?

You may also consider adding some localization strings: Like Copyright, Set copyright etc.. in the lang/xx/ folders.

@codecov
Copy link

codecov bot commented Jun 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.90%. Comparing base (02e5a20) to head (9b332ff).
Report is 51 commits behind head on master.

❗ Current head 9b332ff differs from pull request most recent head 50b87bd. Consider uploading reports for the commit 50b87bd to get more accurate results

Additional details and impacted files

Co-authored-by: Benoît Viguier <ildyria@users.noreply.github.com>
@ThanasisMpalatsoukas
Copy link
Contributor Author

@ildyria
Should i use google translate for the translations that i don't know or should i create the keys on all files and leave them empty?

tested for editing the copyright value. Creating a different
endpoint for removing copyright might not be necessary as the
copyright value can be edited to be "".
@ildyria
Copy link
Member

ildyria commented Jun 19, 2023

@ildyria Should i use google translate for the translations that i don't know or should i create the keys on all files and leave them empty?

add the keys (the tests will require it anyway) and don't bother with google translate, just put the English. If someone wants to do the translation for their language, we will take the PR :)
I will probably make a change on yours to support French (as it is my native language).

lang/fr/lychee.php Outdated Show resolved Hide resolved
Co-authored-by: Benoît Viguier <ildyria@users.noreply.github.com>
lang/fr/lychee.php Outdated Show resolved Hide resolved
@ildyria ildyria added this to the 4.9.4 milestone Jun 20, 2023
@ildyria
Copy link
Member

ildyria commented Jun 24, 2023

@ThanasisMpalatsoukas I fixed the conflicts with master.

We will merge this PR when:

  • tests are added (see above)
  • front-end supports it:
    • You will need to make a PR on Lychee-front for that
    • Once merged, we can rebuild the js files and we will be able to merge this one in master.

😃

@ildyria ildyria modified the milestones: 4.9.4, 4.9.x Jun 25, 2023
@ildyria
Copy link
Member

ildyria commented Jul 10, 2023

@ThanasisMpalatsoukas any updates ? :(

@ThanasisMpalatsoukas
Copy link
Contributor Author

@ildyria Hey, was busy with work and stuff. Will probably take a look at it this week :)

@ildyria
Copy link
Member

ildyria commented Jul 10, 2023

was busy with work and stuff.

I know that feeling quite well. :')

@ildyria ildyria removed this from the 4.1x.0 milestone Aug 21, 2023
@ildyria
Copy link
Member

ildyria commented Aug 21, 2023

@ThanasisMpalatsoukas any updates ? :(

@ARMeeru
Copy link

ARMeeru commented Oct 2, 2023

@ildyria Is it still pending?

@ildyria
Copy link
Member

ildyria commented Oct 2, 2023

Well yes. Those points have still not been resolved.

We will merge this PR when:

  • tests are added (see above)

  • front-end supports it:

    • You will need to make a PR on Lychee-front for that
    • Once merged, we can rebuild the js files and we will be able to merge this one in master.

Though what I would suggest is to fork this Pull request and start integrating against the Livewire branch of #1303 as it will make it significantly easier for the user to add tests and not have to care for the JS front side.

@ARMeeru
Copy link

ARMeeru commented Oct 3, 2023

I'm interested on this. While finalising that interest, I would like to ping @ThanasisMpalatsoukas in case he's on this but forgot.

@ThanasisMpalatsoukas
Copy link
Contributor Author

@ARMeeru thank you for being considerate and pinging me on this, sadly i have too personal work and am unable to continue with this at this point in time. You can go on and finalize this feature :)

@ildyria ildyria changed the title This new feature for adding copyright to albums #1838 [Free to pick up] This new feature for adding copyright to albums #1838 Dec 21, 2023
@ildyria ildyria changed the title [Free to pick up] This new feature for adding copyright to albums #1838 Adding copyright to albums #1838 Apr 28, 2024
Copy link
Member

@ildyria ildyria left a comment

Choose a reason for hiding this comment

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

Tested LGTM.

@ildyria ildyria merged commit 01ca75b into LycheeOrg:master Apr 28, 2024
39 of 45 checks passed
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

Successfully merging this pull request may close these issues.

[Enhancement] Allow setting copyright per picture/album.
3 participants