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

Resolved #3439 where using File field limited to upload directory on other site would result in JavaScript error #3452

Merged
merged 3 commits into from Jul 18, 2023

Conversation

intoeetive
Copy link
Contributor

Resolved #3439 where using File field limited to upload directory on other site would result in JavaScript error

image

@robinsowell please review. We need to test it in different combinations (sites, member role permissions, existing files etc.) to make sure it does not cause data loss

…other site would result in JavaScript error
@intoeetive intoeetive added the Bug: Accepted Bug has been confirmed, is reproducible, and ready to work on. label May 25, 2023
@intoeetive intoeetive added this to the 7.2.18 milestone May 25, 2023
@intoeetive intoeetive marked this pull request as ready for review May 29, 2023 13:27
@intoeetive intoeetive modified the milestones: 7.2.18, 7.3.3 May 29, 2023
@robinsowell
Copy link
Contributor

I'm running into issues, particularly in a matrix field.

I download the branch, did default install. Made a file field called 'blog_file' which is a file field limited to the 'blog' directory. This is all on the default site. Also note, I'm showing version 7.2.17.

I made a second msm site, 'dos'. Made a channel in dos, assigned the 'blog' group and my new 'blog_file' field.

Go to add entry in my dos channel. No js errors- which is good and an improvement. My blog field shows:

Screenshot 2023-06-12 at 10 36 59 AM

That's correct. My grid field could be a bit better on messaging, but I still don't trigger js errors nor can I do a bad upload:

Screenshot 2023-06-12 at 10 37 08 AM

HOWEVER- if I now go make a file upload directory in the dos site, now when I go to add an entry in dos channel, I still don't have js error but my file field looks like:

Screenshot 2023-06-12 at 10 35 25 AM.

If I do drag/drop to it I get:

Screenshot 2023-06-12 at 10 43 04 AM

Weirdly, my matrix field with the file field actually gives me the option of the file field on the dos site:

Screenshot 2023-06-12 at 10 41 28 AM

Which it should not. If I go to the field settings while on dos, it doesn't show any directory selected, which is what happens when the directory chosen isn't available on the site you are currently on:

Screenshot 2023-06-12 at 10 42 33 AM

I had similar issues if I just dropped the updated files into my 7.3 install to test.

Copy link
Contributor

@robinsowell robinsowell left a comment

Choose a reason for hiding this comment

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

See comments, it's doing odd things still.

@intoeetive intoeetive dismissed robinsowell’s stale review June 14, 2023 07:44

Please review again. I'm not getting the kind of Matrix UI you are showing, but it's working without errors for me. The file and Grid fields are working fine - but I'm also wondering if there are edge cases I did not think of

@intoeetive intoeetive modified the milestones: 7.3.5, 7.3.6 Jun 26, 2023
@intoeetive intoeetive modified the milestones: 7.3.6, 7.3.7 Jul 10, 2023
@robinsowell
Copy link
Contributor

@intoeetive I can't get it to break again. I like seeing the debug in the console:

Screenshot 2023-07-10 at 11 27 55 AM

That's useful. I will grant, my head is spinning from all of the possible permutations testing this, but I'm calling it good.

I do still get a radio with nothing selecting in the actual field settings when an unavailable directory is chosen, but I'm going to put that in as a seperate (minor) bug- I think a flag or... something belongs there. Possibly a validation error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Accepted Bug has been confirmed, is reproducible, and ready to work on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File field can break publish page in MSM
3 participants