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

Add range protection #412

Closed
wants to merge 6 commits into from
Closed

Add range protection #412

wants to merge 6 commits into from

Conversation

DavisVaughan
Copy link

@DavisVaughan DavisVaughan commented Aug 17, 2018

This PR implements the protectRange() function. It should be incorporated alongside #411. It protects an Excel range so that when a worksheet is protected with protectWorksheet(), specific ranges are still editable with or without a password specific to that range.

  • There are no checks that the range is in a correct Excel format. If an incorrect format is supplied, upon opening the Excel workbook Excel will complain and ask if you want to remove the faulty XML. If you say yes, it will remove the protected range (in fact I think it removes all of them?).

  • It is meaningful for overlapping ranges to be protected with multiple passwords. See the documentation Details section for more info.

I see this PR being useful when creating data entry templates with R. A data scientist could create a boxed off "holy grail" sheet / range where an analyst can enter data and then pass it back to the data scientist for further munging. The protection prevents the analyst from entering data in cells that could cause the data scientist issues.

CC @kainhofer

kainhofer and others added 6 commits August 16, 2018 21:22
…nxlsx; Add protectWorksheet function to enable worksheet-level protection (potentially password-protected)

This patch loads the <sheetProtection> XML tag from the sheet's xml file, stores it in the corresponding field of the Worksheet and simply writes it out again on saveWorkbook.

To modify the worksheet-level protection, the protectWorksheet function is provided. It can either replace the protection or remove it. It will not modify the flags of the existing worksheet protection.
…otectWorkbook function to set workbook protection
@codecov
Copy link

codecov bot commented Aug 17, 2018

Codecov Report

Merging #412 into master will increase coverage by 0.35%.
The diff coverage is 80.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
+ Coverage   60.23%   60.59%   +0.35%     
==========================================
  Files          30       30              
  Lines        7031     7156     +125     
==========================================
+ Hits         4235     4336     +101     
- Misses       2796     2820      +24
Impacted Files Coverage Δ
R/helperFunctions.R 75.56% <100%> (+0.62%) ⬆️
R/worksheet_class.R 72.93% <50%> (-0.72%) ⬇️
src/load_workbook.cpp 89.85% <66.66%> (-0.3%) ⬇️
R/loadWorkbook.R 69.56% <75%> (-0.02%) ⬇️
R/wrappers.R 44.34% <80.89%> (+2.7%) ⬆️
R/WorkbookClass.R 55.56% <84.61%> (+0.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 025605f...c69f202. Read the comment docs.

@DavisVaughan
Copy link
Author

DavisVaughan commented Aug 17, 2018

Need to refactor on top of #413 rather than #411. Use the hashPassword() helper function. Waiting for that PR to be accepted before doing the refactoring in case anything else changes.

@olangfor
Copy link

Hi,

Sorry to bother you. I noticed there is some work being done (or completed) on protecting the excel sheet. I'm specifically interested in the protectRange functionality. Is there a dev version I use? or better still, when is the release version expected?

Thank you for your great work! Sorry if this information is located elsewhere.

Kind regards,

OIiver

@DavisVaughan
Copy link
Author

DavisVaughan commented Sep 13, 2018

@olangfor I just closed and reopened this in #421 with a proper implementation now that #413 has been merged. Installing that PR should work if you need it immediately.

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