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

Fixes: #2755 Adding privacy for source display to visitors #4748

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dbq-andersons
Copy link

Long-time user, first-time contributor. Please be gentle :-)

In November of 2019, I suggested both in the forums and through a git suggestion (#2755) that I'd like there to be a way to hide source details from visitors and make them available only to members. From my original forum post, "Even though my main site contains only the non-living people I export out of RootsMagic, I'm suddenly not fond of my full source broadcasting. However, I want something there so people don't think my work is sourceless and I'm some kind of noob hack (I do still consider myself a noob hack, but the rest of the world doesn't need to know that)."

My modifications add two new options to the tree privacy page:

2023-02-04_210612

These are a few sources for my dad. This is how they normally look:

2023-02-04_210855

If the source detail toggles are disabled for visitors, they look like this.

2023-02-04_210924

Finally, if the source titles are disabled or visitors, they look like this:

2023-02-04_210948

That's about it. The fact that sources exist can be conveyed to non-authenticated visitors without revealing any detailed information (or any information at all).

Even if these changes don't get accepted into Webtrees proper, no worries, it was a good coding experience dabbling into .phtml files for the first time and learning the submission process.

Cheers,

Bill

dbq-andersons and others added 3 commits February 4, 2023 21:04
Remove extra leading tabs from lines 116, 117, 127, and 128.
Replaced tab stops with spaces on lines 116,117, 127, and 128.

Removed extra tab stops on line 118
@codecov
Copy link

codecov bot commented Feb 7, 2023

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 30.92%. Comparing base (71e7c06) to head (7c75893).
Report is 506 commits behind head on main.

❗ Current head 7c75893 differs from pull request most recent head 065eef0. Consider uploading reports for the commit 065eef0 to get more accurate results

Files Patch % Lines
app/Http/RequestHandlers/TreePrivacyAction.php 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4748      +/-   ##
============================================
- Coverage     30.92%   30.92%   -0.01%     
  Complexity    11361    11361              
============================================
  Files          1188     1174      -14     
  Lines         47909    47896      -13     
============================================
- Hits          14817    14812       -5     
+ Misses        33092    33084       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbq-andersons
Copy link
Author

I'm at a little of a loss here mostly because I don't know much about codecov and what exactly it's looking for. I ran the tests on my side through PHPUnit and those came back 100% clean (see attached at the end of this post). So, I guess I'm looking for guidance as to how to turn the codecov checks green. Magic? Bribary? Bag of non-sequential unmarked bills under a park bench?

Any advice would be greatly appreciated.

Thanks,

Bill

anderson@steve:/tools/webdocs/genealogy/webtrees2-dev$ vendor/bin/phpunit
PHPUnit 9.5.27 by Sebastian Bergmann and contributors.

.............................................................   61 / 3182 (  1%)
.............................................................  122 / 3182 (  3%)
.............................................................  183 / 3182 (  5%)
.............................................................  244 / 3182 (  7%)
.............................................................  305 / 3182 (  9%)
.............................................................  366 / 3182 ( 11%)
.............................................................  427 / 3182 ( 13%)
.............................................................  488 / 3182 ( 15%)
.............................................................  549 / 3182 ( 17%)
.............................................................  610 / 3182 ( 19%)
.............................................................  671 / 3182 ( 21%)
.............................................................  732 / 3182 ( 23%)
.............................................................  793 / 3182 ( 24%)
.............................................................  854 / 3182 ( 26%)
.............................................................  915 / 3182 ( 28%)
.............................................................  976 / 3182 ( 30%)
............................................................. 1037 / 3182 ( 32%)
............................................................. 1098 / 3182 ( 34%)
............................................................. 1159 / 3182 ( 36%)
............................................................. 1220 / 3182 ( 38%)
............................................................. 1281 / 3182 ( 40%)
............................................................. 1342 / 3182 ( 42%)
............................................................. 1403 / 3182 ( 44%)
............................................................. 1464 / 3182 ( 46%)
............................................................. 1525 / 3182 ( 47%)
............................................................. 1586 / 3182 ( 49%)
............................................................. 1647 / 3182 ( 51%)
............................................................. 1708 / 3182 ( 53%)
............................................................. 1769 / 3182 ( 55%)
............................................................. 1830 / 3182 ( 57%)
............................................................. 1891 / 3182 ( 59%)
............................................................. 1952 / 3182 ( 61%)
............................................................. 2013 / 3182 ( 63%)
............................................................. 2074 / 3182 ( 65%)
............................................................. 2135 / 3182 ( 67%)
............................................................. 2196 / 3182 ( 69%)
............................................................. 2257 / 3182 ( 70%)
............................................................. 2318 / 3182 ( 72%)
............................................................. 2379 / 3182 ( 74%)
............................................................. 2440 / 3182 ( 76%)
............................................................. 2501 / 3182 ( 78%)
............................................................. 2562 / 3182 ( 80%)
............................................................. 2623 / 3182 ( 82%)
............................................................. 2684 / 3182 ( 84%)
............................................................. 2745 / 3182 ( 86%)
............................................................. 2806 / 3182 ( 88%)
............................................................. 2867 / 3182 ( 90%)
............................................................. 2928 / 3182 ( 92%)
............................................................. 2989 / 3182 ( 93%)
............................................................. 3050 / 3182 ( 95%)
............................................................. 3111 / 3182 ( 97%)
............................................................. 3172 / 3182 ( 99%)
..........                                                    3182 / 3182 (100%)

Time: 00:11.612, Memory: 174.50 MB

OK (3182 tests, 144104 assertions)
anderson@steve:/tools/webdocs/genealogy/webtrees2-dev$

@fisharebest
Copy link
Owner

Hi Bill.

Sorry for not replying earlier. Too many things happening IRL at the moment.

CodeCov just counts the lines of code that get used one or more times when you run the tests.
(In an ideal world, we'd have 100% coverage - and all new code would come with tests.)
The output just says that you have added code without tests, and this reduces the coverage
by a tiny fraction of a percent. You can ignore this.

TIP: if the subject line contains the text Fixes: #1234, then accepting the PR will automatically
close the corresponding issue, and everything gets linked together nicely.

Reviewing PRs often takes more time that writing them. I tend to spend too long worrying about
the exact wording of any new text/translations, and how I can use the change as an opportunity
to improve existing code. For example, I might make this logic available to all object types, but
only add configuration options for sources.

So be patient if I appear to be ignoring you.

@dbq-andersons
Copy link
Author

Thanks, Greg for taking the time to reply and for your patience with a beginner in this process. Everything you have said makes sense; I appreciate you taking the time to check out what I've written.

Cheers!

Bill

@dbq-andersons dbq-andersons changed the title Adding privacy for source display to visitors Fixes: #2755 Adding privacy for source display to visitors Feb 9, 2023
Copy link
Contributor

@HerzScheisse HerzScheisse left a comment

Choose a reason for hiding this comment

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

Thanks for you PR, i added it to my site too and really like your additions... one small suggestion:

<?php if (Auth::isMember($tree) || $tree->getPreference('SHOW_SOURCE_TITLES') === '0') : ?>
<?php $value = '<span class="field" dir="auto"><a href="' . e($source->url()) . '">' . $source->fullName() . '</a></span>' ?>
<?php else: ?>
<?php $value = '<span class="field" dir="auto">' . I18N::translate('PRIVATE') . '</span>' ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

if you use I18N::translate('Private') instead of I18N::translate('PRIVATE') the existing translations to other langguages will be successful ;-)

@dbq-andersons
Copy link
Author

Thanks @HerzScheisse for the tip! I just today saw your comment from 12 February; I haven't looked at my Webtrees stuff for quite a while (my site is still running 2.1.16 or crying out loud!). I should bite the bullet and upgrade to 2.1.19 and then reapply the changes.

I'm glad you liked the changes I made in the PR. Hopefully, they can help others as well someday

Cheers!

Bill

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.

None yet

3 participants